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
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:
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
Hi all,Assuming Info is a valid pointer, Info->VolumeLabel is an array object.
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 ?
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,
So if (Info->VolumeLabel == NULL) will always evaluate to FALSE when Info != NULL. Can we just remove it.
Thanks
Wenyi
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.
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 ?
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
.
Laszlo Ersek
On 11/24/20 12:05, xiewenyi (A) wrote:
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
Hi Laszlo,Well, we can approach this on two levels.
So if (Info->VolumeLabel == NULL) will always evaluate to FALSE when Info != NULL. Can we just remove it.
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.
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 ?
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,
Thank you for your detailed explanation, I get it.
I will post a patch later.
Thanks
Wenyi
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.
So if (Info->VolumeLabel == NULL) will always evaluate to FALSE when Info != NULL. Can we just remove it.
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!
LaszloOn 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.
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 ?
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
.