On 2017-10-17 02:15 PM, Christian König wrote:
> Am 17.10.2017 um 19:58 schrieb Felix Kuehling:
>> On 2017-10-17 01:25 PM, Tom St Denis wrote:
>>> On 17/10/17 01:23 PM, Tom St Denis wrote:
>>>> On 17/10/17 01:18 PM, Christian König wrote:
>>>>> Am 17.10.2017 um 16:10 schrieb Tom St Denis:
>>>>>> In this block of code:
>>>>>>
>>>>>> void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector)
>>>>>> {
>>>>>> Â Â Â Â struct dm_connector_state *state =
>>>>>> Â Â Â Â Â Â Â Â to_dm_connector_state(connector->state);
>>>>>>
>>>>>> Â Â Â Â kfree(state);
>>>>>>
>>>>>> Â Â Â Â state = kzalloc(sizeof(*state), GFP_KERNEL);
>>>>>>
>>>>>>
>>>>>> The value of state is never compared with NULL and moreso the value
>>>>>> of connector->state is never written to if NULL. Wouldn't this mean
>>>>>> the pointer points to freed memory?
>>> Sorry I think I might be explaining this poorly.
>>>
>>> In the case the alloc succeeds the pointer is updated and everything
>>> is fine.
>>>
>>> IF the alloc fails the pointer (connector->state) is not updated and
>>> the value points to freed memory.
>> I'm wondering why the function frees, and then reallocates the memory.
>> Does its size change? If not, why not just memset it to 0?
>
> Yeah, I was just thinking the same thing.
>
> And no from the code snippet Tom posted it just frees and reallocated the same structure.
>
> I think that should rather look like this:
>
> if (!state)
> Â Â Â Â state = kzalloc(...);
> else
> Â Â Â memset(...);
>
I don't expect connector->state to ever be NULL here which seems to be confirmed by
code that would have blown up if it ever was. How about we do a BUG_ON on this condition
as has been suggested?
Otherwise I like the memset idea. No need for free/alloc here.
Harry
> Regards,
> Christian.
>
>>
>> Regards,
>> Â Â Felix
>>
>>> Tom
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
