Date
1 - 5 of 5
redundant NULL pointer check
wenyi,xie
Hi all,
When removing -Wno-tautological-compare, there's a warning. MdeModulePkg/Library/FileExplorerLib/FileExplorer.c:850:19: warning: comparison of array 'Info->VolumeLabel' equal to a null pointer is always false [-Wtautological-pointer-compare] Search "no-tautological-compare" (4904 hits in 1 file) Since Info is a pointer of struct EFI_FILE_SYSTEM_VOLUME_LABEL, and this struct has only one member VolumeLabel. So Info and Info->VolumeLabel are point to the same place. So if Info != NULL, is it necessary to check Info->VolumeLabel == NULL again ? Thanks Wenyi |
|
Laszlo Ersek
On 11/24/20 10:22, wenyi,xie via groups.io wrote:
Hi all,Assuming Info is a valid pointer, Info->VolumeLabel is an array object. When evaluated, Info->VolumeLabel is converted to the address of its first element. The address of the first element of an array object can never be NULL. And so (Info->VolumeLabel == NULL) will always evaluate to FALSE. Thanks Laszlo |
|
wenyi,xie
Hi Laszlo,
toggle quoted message
Show quoted text
So if (Info->VolumeLabel == NULL) will always evaluate to FALSE when Info != NULL. Can we just remove it. Thanks Wenyi On 2020/11/24 18:52, Laszlo Ersek wrote:
On 11/24/20 10:22, wenyi,xie via groups.io wrote:Hi all,Assuming Info is a valid pointer, Info->VolumeLabel is an array object. |
|
Laszlo Ersek
On 11/24/20 12:05, xiewenyi (A) wrote:
Hi Laszlo,Well, we can approach this on two levels. The first level is whether the code transformation (the patch) that you are describing above would be correct. Yes, removing the comparison in question is correct. Please feel free to post the patch. But the deeper (zeroth) level is more nuanced. Namely, it's not specifically the *non-nullity* of "Info" that guarantees (Info->VolumeLabel != NULL). Instead, that consequence originates from the *validity* of Info (namely, that it points to an EFI_FILE_SYSTEM_VOLUME_LABEL object), and from the fact that "VolumeLabel" is an *array* in EFI_FILE_SYSTEM_VOLUME_LABEL. The address of an array object (well, of any object) that *exists* can never be NULL. I'm not sure if my depiction is understandable. Basically, if Info is not NULL, then (Info->VolumeLabel != NULL) holds *exactly because*: CHAR16 VolumeLabel[1]; ASSERT (&VolumeLabel[0] != NULL); holds as well. Anyway... feel free to post the patch. Thanks! Laszlo On 2020/11/24 18:52, Laszlo Ersek wrote:On 11/24/20 10:22, wenyi,xie via groups.io wrote:Hi all,Assuming Info is a valid pointer, Info->VolumeLabel is an array object. |
|
wenyi,xie
Hi Laszlo,
toggle quoted message
Show quoted text
Thank you for your detailed explanation, I get it. I will post a patch later. Thanks Wenyi On 2020/11/25 6:56, Laszlo Ersek wrote:
On 11/24/20 12:05, xiewenyi (A) wrote:Hi Laszlo,Well, we can approach this on two levels. |
|