On 07/06/17 08:12 PM, Emil Velikov wrote:
> On 7 June 2017 at 09:40, Michel Dänzer <michel at daenzer.net> wrote:
>> On 06/06/17 10:43 PM, Emil Velikov wrote:
>>> On 31 May 2017 at 21:22, Samuel Li <Samuel.Li at amd.com> wrote:
>>>
>>>> --- /dev/null
>>>> +++ b/amdgpu/amdgpu_asic_id.c
>>>
>>>> +static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
>>>> +{
>>>> + char *buf, *saveptr;
>>>> + char *s_did;
>>>> + char *s_rid;
>>>> + char *s_name;
>>>> + char *endptr;
>>>> + int r = 0;
>>>> +
>>>> + buf = strdup(line);
>>> You don't need the extra strdup here if you use strchr over strtok.
>>
>> Beware that without strdup here, amdgpu_parse_asic_ids must set line =
>> NULL after table_line++, so that getline allocates a new buffer for the
>> next line.
>>
> A simple "line = NULL" will lead to a memory leak, AFAICT.
>
> In either case, I'm a bit baffled how that is affected by the
> presence/lack of strdup?
> We don't alter or reuse the backing storage only
> strcmp/isblank/strtol/strdup it.
Oh, I missed that id->marketing_name is strdup'd again.
Anyway, it's probably better not to change the logic too much at this
point, other than anything needed to fix immediate bugs. It can always
be improved with follow-up patches.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
