[sheepdog] [PATCH] dog: don't free value if it is passed via command line

[Hitoshi Mitake]

At Fri, 15 Aug 2014 10:26:42 +0800,
Ruoyu wrote:
> 
> 
> On 2014年08月15日 09:32, Hitoshi Mitake wrote:
> > vdi_setattr() has two way of obtaining value of attr:
> > 1. command line parameter
> > 2. read from stdin
> >
> > In a case of 1, the value shouldn't be freed because it is not in heap
> > area. But current dog does it. This patch removes this invalid free.
> In case 1, what about value = strdup(argv[optind++]) to alloc the space 
> in stack area?
> Then, free will fit for both situations.
> Because I think the new variable value_allocated is confused for developers.

I want to avoid using strdup() in this case. Because strdup() can fail
and we need to add another error handling code. It would add another
complexity.

How about enhancing comment for the new variable (value_allocated)?

Thanks,
Hitoshi

> >
> > Reported-by: Ruoyu <liangry at ucweb.com>
> > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > ---
> >   dog/vdi.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/dog/vdi.c b/dog/vdi.c
> > index 84715b3..1d8cc6e 100644
> > --- a/dog/vdi.c
> > +++ b/dog/vdi.c
> > @@ -1278,6 +1278,7 @@ static int vdi_setattr(int argc, char **argv)
> >   	const char *vdiname = argv[optind++], *key;
> >   	char *value = NULL;
> >   	uint64_t offset;
> > +	bool value_alloced = false;
> >   
> >   	key = argv[optind++];
> >   	if (!key) {
> > @@ -1289,6 +1290,7 @@ static int vdi_setattr(int argc, char **argv)
> >   	value = argv[optind++];
> >   	if (!value && !vdi_cmd_data.delete) {
> >   		value = xmalloc(SD_MAX_VDI_ATTR_VALUE_LEN);
> > +		value_alloced = true;
> >   
> >   		offset = 0;
> >   reread:
> > @@ -1333,7 +1335,7 @@ reread:
> >   	}
> >   
> >   out:
> > -	if (value)
> > +	if (value_alloced)
> >   		free(value);
> >   
> >   	return ret;
> 
> 
> -- 
> sheepdog mailing list
> sheepdog at lists.wpkg.org
> http://lists.wpkg.org/mailman/listinfo/sheepdog