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 <lian...@ucweb.com>
>>> Signed-off-by: Hitoshi Mitake <mitake.hito...@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@lists.wpkg.org
>> http://lists.wpkg.org/mailman/listinfo/sheepdog
--
sheepdog mailing list
sheepdog@lists.wpkg.org
http://lists.wpkg.org/mailman/listinfo/sheepdog
