Re: [PATCH v1 1/2] MdeModulePkg/NetLib.h: Fix the possible NULL pointer dereference issue.

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Hi Laszlo,

Thanks review the patch.

> 
> (1) The linked list from BaseLib (LIST_ENTRY) always has at least one
> element (the head element), and the list is empty if the head element
> points back to itself. In other words, ForwardLink may never be NULL.
> 
> So why is it necessary to check against that case here?
> 

I agree the ForwardLink in valid LIST_ENTRY can't be NULL. Here, I added the more condition check was considering the possible broken case of the LIST_ENTRY. But I also checked the other usage of *_FOR_EACH_SAFE /*_FOR_EACH, looks never or unnecessary to consider that case.

If so,  please drop the series patches and I will create another series patches to remove the unnecessary check after retrieving the value from list Entry.


> (2) If the NULL check is indeed necessary for some reason, then we
> should write
> 
>   Entry != (ListHead) && Entry != NULL
> 
> in the controlling expression. Because, with the comma operator, the
> (Entry != (ListHead)) expression would be evaluated, but its result
> would be ignored.
> 

Yes, I was also awared that, so I created patch v2 to fix that: 

[edk2] [PATCH v2 1/2] MdeModulePkg/NetLib.h: Fix the possible NULL pointer dereference issue.
Fix the wrong condition in for cycle.

Thanks,
Jiaxin