In edk2, initialization of local variables is forbidden, both for
stylistic reasons and because such initialization may generate calls to
compiler intrinsics.
For the following initialization in UdfRead():
CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
clang-3.8 generates a memset() call, when building UdfDxe for IA32, which
then fails to link.
Replace the initialization with ZeroMem().
Do the same to "FilePath" in UdfOpen().
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 8b9339567f8e..e7159ff861f7 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -174,15 +174,16 @@ UdfOpen (
{
EFI_TPL OldTpl;
EFI_STATUS Status;
PRIVATE_UDF_FILE_DATA *PrivFileData;
PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
- CHAR16 FilePath[UDF_PATH_LENGTH] = { 0 };
+ CHAR16 FilePath[UDF_PATH_LENGTH];
UDF_FILE_INFO File;
PRIVATE_UDF_FILE_DATA *NewPrivFileData;
CHAR16 *TempFileName;
+ ZeroMem (FilePath, sizeof FilePath);
OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
if (This == NULL || NewHandle == NULL || FileName == NULL) {
Status = EFI_INVALID_PARAMETER;
goto Error_Invalid_Params;
@@ -322,14 +323,15 @@ UdfRead (
EFI_BLOCK_IO_PROTOCOL *BlockIo;
EFI_DISK_IO_PROTOCOL *DiskIo;
UDF_FILE_INFO FoundFile;
UDF_FILE_IDENTIFIER_DESCRIPTOR *NewFileIdentifierDesc;
VOID *NewFileEntryData;
- CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
+ CHAR16 FileName[UDF_FILENAME_LENGTH];
UINT64 FileSize;
UINT64 BufferSizeUint64;
+ ZeroMem (FileName, sizeof FileName);
OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
if (This == NULL || BufferSize == NULL || (*BufferSize != 0 &&
Buffer == NULL)) {
Status = EFI_INVALID_PARAMETER;
--
2.14.1.3.gb7cf6e02401b
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Reviewed-by: Star Zeng <star.zeng@intel.com>
BTW: How about to use "sizeof ()" instead of "sizeof"?
Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Sunday, September 10, 2017 8:13 AM
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Dong, Eric <eric.dong@intel.com>; Paulo Alcantara <pcacjr@zytor.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH 3/5] MdeModulePkg/UdfDxe: replace zero-init of local variables with ZeroMem()
In edk2, initialization of local variables is forbidden, both for stylistic reasons and because such initialization may generate calls to compiler intrinsics.
For the following initialization in UdfRead():
CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
clang-3.8 generates a memset() call, when building UdfDxe for IA32, which then fails to link.
Replace the initialization with ZeroMem().
Do the same to "FilePath" in UdfOpen().
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 8b9339567f8e..e7159ff861f7 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -174,15 +174,16 @@ UdfOpen (
{
EFI_TPL OldTpl;
EFI_STATUS Status;
PRIVATE_UDF_FILE_DATA *PrivFileData;
PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
- CHAR16 FilePath[UDF_PATH_LENGTH] = { 0 };
+ CHAR16 FilePath[UDF_PATH_LENGTH];
UDF_FILE_INFO File;
PRIVATE_UDF_FILE_DATA *NewPrivFileData;
CHAR16 *TempFileName;
+ ZeroMem (FilePath, sizeof FilePath);
OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
if (This == NULL || NewHandle == NULL || FileName == NULL) {
Status = EFI_INVALID_PARAMETER;
goto Error_Invalid_Params;
@@ -322,14 +323,15 @@ UdfRead (
EFI_BLOCK_IO_PROTOCOL *BlockIo;
EFI_DISK_IO_PROTOCOL *DiskIo;
UDF_FILE_INFO FoundFile;
UDF_FILE_IDENTIFIER_DESCRIPTOR *NewFileIdentifierDesc;
VOID *NewFileEntryData;
- CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
+ CHAR16 FileName[UDF_FILENAME_LENGTH];
UINT64 FileSize;
UINT64 BufferSizeUint64;
+ ZeroMem (FileName, sizeof FileName);
OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
if (This == NULL || BufferSize == NULL || (*BufferSize != 0 &&
Buffer == NULL)) {
Status = EFI_INVALID_PARAMETER;
--
2.14.1.3.gb7cf6e02401b
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Star,
Sizeof is an operator, not a function, like + or -. Not having () is ok.
Thanks/Ray
> -----Original Message-----
> From: Zeng, Star
> Sent: Tuesday, September 12, 2017 4:55 PM
> To: Laszlo Ersek <lersek@redhat.com>; edk2-devel-01 <edk2-
> devel@lists.01.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Dong, Eric
> <eric.dong@intel.com>; Paulo Alcantara <pcacjr@zytor.com>; Ni, Ruiyu
> <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH 3/5] MdeModulePkg/UdfDxe: replace zero-init of local
> variables with ZeroMem()
>
> Reviewed-by: Star Zeng <star.zeng@intel.com>
>
> BTW: How about to use "sizeof ()" instead of "sizeof"?
>
>
> Thanks,
> Star
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Sunday, September 10, 2017 8:13 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Dong, Eric
> <eric.dong@intel.com>; Paulo Alcantara <pcacjr@zytor.com>; Ni, Ruiyu
> <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH 3/5] MdeModulePkg/UdfDxe: replace zero-init of local
> variables with ZeroMem()
>
> In edk2, initialization of local variables is forbidden, both for stylistic reasons
> and because such initialization may generate calls to compiler intrinsics.
>
> For the following initialization in UdfRead():
>
> CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
>
> clang-3.8 generates a memset() call, when building UdfDxe for IA32, which
> then fails to link.
>
> Replace the initialization with ZeroMem().
>
> Do the same to "FilePath" in UdfOpen().
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Paulo Alcantara <pcacjr@zytor.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> index 8b9339567f8e..e7159ff861f7 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> @@ -174,15 +174,16 @@ UdfOpen (
> {
> EFI_TPL OldTpl;
> EFI_STATUS Status;
> PRIVATE_UDF_FILE_DATA *PrivFileData;
> PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
> - CHAR16 FilePath[UDF_PATH_LENGTH] = { 0 };
> + CHAR16 FilePath[UDF_PATH_LENGTH];
> UDF_FILE_INFO File;
> PRIVATE_UDF_FILE_DATA *NewPrivFileData;
> CHAR16 *TempFileName;
>
> + ZeroMem (FilePath, sizeof FilePath);
> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>
> if (This == NULL || NewHandle == NULL || FileName == NULL) {
> Status = EFI_INVALID_PARAMETER;
> goto Error_Invalid_Params;
> @@ -322,14 +323,15 @@ UdfRead (
> EFI_BLOCK_IO_PROTOCOL *BlockIo;
> EFI_DISK_IO_PROTOCOL *DiskIo;
> UDF_FILE_INFO FoundFile;
> UDF_FILE_IDENTIFIER_DESCRIPTOR *NewFileIdentifierDesc;
> VOID *NewFileEntryData;
> - CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
> + CHAR16 FileName[UDF_FILENAME_LENGTH];
> UINT64 FileSize;
> UINT64 BufferSizeUint64;
>
> + ZeroMem (FileName, sizeof FileName);
> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>
> if (This == NULL || BufferSize == NULL || (*BufferSize != 0 &&
> Buffer == NULL)) {
> Status = EFI_INVALID_PARAMETER;
> --
> 2.14.1.3.gb7cf6e02401b
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 09/12/17 11:38, Ni, Ruiyu wrote:
> Star,
> Sizeof is an operator, not a function, like + or -. Not having () is ok.
Ugh, just seeing this now :) So what should I do now?
If Star agrees, I would prefer *not* to add the parens. If Star insists,
I can add them.
Thanks
Laszlo
>> -----Original Message-----
>> From: Zeng, Star
>> Sent: Tuesday, September 12, 2017 4:55 PM
>> To: Laszlo Ersek <lersek@redhat.com>; edk2-devel-01 <edk2-
>> devel@lists.01.org>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Dong, Eric
>> <eric.dong@intel.com>; Paulo Alcantara <pcacjr@zytor.com>; Ni, Ruiyu
>> <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: RE: [PATCH 3/5] MdeModulePkg/UdfDxe: replace zero-init of local
>> variables with ZeroMem()
>>
>> Reviewed-by: Star Zeng <star.zeng@intel.com>
>>
>> BTW: How about to use "sizeof ()" instead of "sizeof"?
>>
>>
>> Thanks,
>> Star
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Sunday, September 10, 2017 8:13 AM
>> To: edk2-devel-01 <edk2-devel@lists.01.org>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Dong, Eric
>> <eric.dong@intel.com>; Paulo Alcantara <pcacjr@zytor.com>; Ni, Ruiyu
>> <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: [PATCH 3/5] MdeModulePkg/UdfDxe: replace zero-init of local
>> variables with ZeroMem()
>>
>> In edk2, initialization of local variables is forbidden, both for stylistic reasons
>> and because such initialization may generate calls to compiler intrinsics.
>>
>> For the following initialization in UdfRead():
>>
>> CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
>>
>> clang-3.8 generates a memset() call, when building UdfDxe for IA32, which
>> then fails to link.
>>
>> Replace the initialization with ZeroMem().
>>
>> Do the same to "FilePath" in UdfOpen().
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Paulo Alcantara <pcacjr@zytor.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> index 8b9339567f8e..e7159ff861f7 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> @@ -174,15 +174,16 @@ UdfOpen (
>> {
>> EFI_TPL OldTpl;
>> EFI_STATUS Status;
>> PRIVATE_UDF_FILE_DATA *PrivFileData;
>> PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
>> - CHAR16 FilePath[UDF_PATH_LENGTH] = { 0 };
>> + CHAR16 FilePath[UDF_PATH_LENGTH];
>> UDF_FILE_INFO File;
>> PRIVATE_UDF_FILE_DATA *NewPrivFileData;
>> CHAR16 *TempFileName;
>>
>> + ZeroMem (FilePath, sizeof FilePath);
>> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>>
>> if (This == NULL || NewHandle == NULL || FileName == NULL) {
>> Status = EFI_INVALID_PARAMETER;
>> goto Error_Invalid_Params;
>> @@ -322,14 +323,15 @@ UdfRead (
>> EFI_BLOCK_IO_PROTOCOL *BlockIo;
>> EFI_DISK_IO_PROTOCOL *DiskIo;
>> UDF_FILE_INFO FoundFile;
>> UDF_FILE_IDENTIFIER_DESCRIPTOR *NewFileIdentifierDesc;
>> VOID *NewFileEntryData;
>> - CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
>> + CHAR16 FileName[UDF_FILENAME_LENGTH];
>> UINT64 FileSize;
>> UINT64 BufferSizeUint64;
>>
>> + ZeroMem (FileName, sizeof FileName);
>> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>>
>> if (This == NULL || BufferSize == NULL || (*BufferSize != 0 &&
>> Buffer == NULL)) {
>> Status = EFI_INVALID_PARAMETER;
>> --
>> 2.14.1.3.gb7cf6e02401b
>>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
I know sizeof works same with sizeof () here, I have no preference, that's why I gave Reviewed-by before the comment, I just see most of the code are using sizeof (), I am not sure whether there is a code standard for it somewhere, I guess no.
Anyway, you can make the decision when pushing. :)
Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Tuesday, September 12, 2017 5:57 PM
To: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Dong, Eric <eric.dong@intel.com>; Paulo Alcantara <pcacjr@zytor.com>
Subject: Re: [PATCH 3/5] MdeModulePkg/UdfDxe: replace zero-init of local variables with ZeroMem()
On 09/12/17 11:38, Ni, Ruiyu wrote:
> Star,
> Sizeof is an operator, not a function, like + or -. Not having () is ok.
Ugh, just seeing this now :) So what should I do now?
If Star agrees, I would prefer *not* to add the parens. If Star insists, I can add them.
Thanks
Laszlo
>> -----Original Message-----
>> From: Zeng, Star
>> Sent: Tuesday, September 12, 2017 4:55 PM
>> To: Laszlo Ersek <lersek@redhat.com>; edk2-devel-01 <edk2-
>> devel@lists.01.org>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Dong, Eric
>> <eric.dong@intel.com>; Paulo Alcantara <pcacjr@zytor.com>; Ni, Ruiyu
>> <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: RE: [PATCH 3/5] MdeModulePkg/UdfDxe: replace zero-init of
>> local variables with ZeroMem()
>>
>> Reviewed-by: Star Zeng <star.zeng@intel.com>
>>
>> BTW: How about to use "sizeof ()" instead of "sizeof"?
>>
>>
>> Thanks,
>> Star
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Sunday, September 10, 2017 8:13 AM
>> To: edk2-devel-01 <edk2-devel@lists.01.org>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Dong, Eric
>> <eric.dong@intel.com>; Paulo Alcantara <pcacjr@zytor.com>; Ni, Ruiyu
>> <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: [PATCH 3/5] MdeModulePkg/UdfDxe: replace zero-init of local
>> variables with ZeroMem()
>>
>> In edk2, initialization of local variables is forbidden, both for
>> stylistic reasons and because such initialization may generate calls to compiler intrinsics.
>>
>> For the following initialization in UdfRead():
>>
>> CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
>>
>> clang-3.8 generates a memset() call, when building UdfDxe for IA32,
>> which then fails to link.
>>
>> Replace the initialization with ZeroMem().
>>
>> Do the same to "FilePath" in UdfOpen().
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Paulo Alcantara <pcacjr@zytor.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> index 8b9339567f8e..e7159ff861f7 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> @@ -174,15 +174,16 @@ UdfOpen (
>> {
>> EFI_TPL OldTpl;
>> EFI_STATUS Status;
>> PRIVATE_UDF_FILE_DATA *PrivFileData;
>> PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
>> - CHAR16 FilePath[UDF_PATH_LENGTH] = { 0 };
>> + CHAR16 FilePath[UDF_PATH_LENGTH];
>> UDF_FILE_INFO File;
>> PRIVATE_UDF_FILE_DATA *NewPrivFileData;
>> CHAR16 *TempFileName;
>>
>> + ZeroMem (FilePath, sizeof FilePath);
>> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>>
>> if (This == NULL || NewHandle == NULL || FileName == NULL) {
>> Status = EFI_INVALID_PARAMETER;
>> goto Error_Invalid_Params;
>> @@ -322,14 +323,15 @@ UdfRead (
>> EFI_BLOCK_IO_PROTOCOL *BlockIo;
>> EFI_DISK_IO_PROTOCOL *DiskIo;
>> UDF_FILE_INFO FoundFile;
>> UDF_FILE_IDENTIFIER_DESCRIPTOR *NewFileIdentifierDesc;
>> VOID *NewFileEntryData;
>> - CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
>> + CHAR16 FileName[UDF_FILENAME_LENGTH];
>> UINT64 FileSize;
>> UINT64 BufferSizeUint64;
>>
>> + ZeroMem (FileName, sizeof FileName);
>> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>>
>> if (This == NULL || BufferSize == NULL || (*BufferSize != 0 &&
>> Buffer == NULL)) {
>> Status = EFI_INVALID_PARAMETER;
>> --
>> 2.14.1.3.gb7cf6e02401b
>>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 09/12/17 10:55, Zeng, Star wrote:
> Reviewed-by: Star Zeng <star.zeng@intel.com>
>
> BTW: How about to use "sizeof ()" instead of "sizeof"?
"sizeof" is a unary operator. The parentheses are mandatory when sizeof
is used with a type name, but when the operand of sizeof is an
expression, the parentheses frequently make the wrong impression.
Consider for example
UINTN Var1, Var2, Var3;
STRUCTURE_TYPE VarStruct;
Var1 = 5;
Var2 = sizeof Var3 + Var1; // [1]
Most people dislike the assignment to Var2, and will rewrite it as:
Var2 = sizeof (Var3) + Var1; // [2]
However, this is totally irrelevant, and does not reflect the binding
strengths between the "sizeof" and "+" operators. In order to reflect
that relationship correctly, the following parentheses should be added:
Var2 = (sizeof Var3) + Var1; // [3]
None of [2] or [3] make any difference relative to [1], of course, in
behavior.
I just dislike [2] because it *suggests* that putting Var3 in parens
"protects" Var1 from falling under "sizeof".
That's not the case. "sizeof" is not a function, it is a unary operator
like "*", "&", "!", etc.
If we want to add parens for emphasizing the actual operator binding,
then [3] is the way to go. For example, if "Var3" was a pointer, you
wouldn't write
*(Var3) + Var1
in order to emphasize that "*" takes precedence over "+"; you would write
(*Var3) + Var1
TL;DR: "sizeof" is a unary, prefix operator, not a function designator.
... In order not to delay this any longer, I will add the parens before
I push.
But, we should all know that those parens don't mean what most people
think they mean :)
Laszlo
>
>
> Thanks,
> Star
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Sunday, September 10, 2017 8:13 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Dong, Eric <eric.dong@intel.com>; Paulo Alcantara <pcacjr@zytor.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH 3/5] MdeModulePkg/UdfDxe: replace zero-init of local variables with ZeroMem()
>
> In edk2, initialization of local variables is forbidden, both for stylistic reasons and because such initialization may generate calls to compiler intrinsics.
>
> For the following initialization in UdfRead():
>
> CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
>
> clang-3.8 generates a memset() call, when building UdfDxe for IA32, which then fails to link.
>
> Replace the initialization with ZeroMem().
>
> Do the same to "FilePath" in UdfOpen().
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Paulo Alcantara <pcacjr@zytor.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> index 8b9339567f8e..e7159ff861f7 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> @@ -174,15 +174,16 @@ UdfOpen (
> {
> EFI_TPL OldTpl;
> EFI_STATUS Status;
> PRIVATE_UDF_FILE_DATA *PrivFileData;
> PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
> - CHAR16 FilePath[UDF_PATH_LENGTH] = { 0 };
> + CHAR16 FilePath[UDF_PATH_LENGTH];
> UDF_FILE_INFO File;
> PRIVATE_UDF_FILE_DATA *NewPrivFileData;
> CHAR16 *TempFileName;
>
> + ZeroMem (FilePath, sizeof FilePath);
> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>
> if (This == NULL || NewHandle == NULL || FileName == NULL) {
> Status = EFI_INVALID_PARAMETER;
> goto Error_Invalid_Params;
> @@ -322,14 +323,15 @@ UdfRead (
> EFI_BLOCK_IO_PROTOCOL *BlockIo;
> EFI_DISK_IO_PROTOCOL *DiskIo;
> UDF_FILE_INFO FoundFile;
> UDF_FILE_IDENTIFIER_DESCRIPTOR *NewFileIdentifierDesc;
> VOID *NewFileEntryData;
> - CHAR16 FileName[UDF_FILENAME_LENGTH] = { 0 };
> + CHAR16 FileName[UDF_FILENAME_LENGTH];
> UINT64 FileSize;
> UINT64 BufferSizeUint64;
>
> + ZeroMem (FileName, sizeof FileName);
> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>
> if (This == NULL || BufferSize == NULL || (*BufferSize != 0 &&
> Buffer == NULL)) {
> Status = EFI_INVALID_PARAMETER;
> --
> 2.14.1.3.gb7cf6e02401b
>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.