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,
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 ?
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,

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,
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 ?
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

.


Laszlo Ersek
 

On 11/24/20 12:05, xiewenyi (A) wrote:
Hi Laszlo,

So if (Info->VolumeLabel == NULL) will always evaluate to FALSE when Info != NULL. Can we just remove it.
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,
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 ?
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,

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,

So if (Info->VolumeLabel == NULL) will always evaluate to FALSE when Info != NULL. Can we just remove it.
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,
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 ?
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

.
.