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

[Ruoyu]

On 2014年08月18日 10:26, Hitoshi Mitake wrote:
> 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 adding a new function xstrdup with error handling bundled,
similar to xmalloc?
>
> How about enhancing comment for the new variable (value_allocated)?
I hope the code is self-explained as far as possible.
>
> 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