On 09/12/17 10:55, Zeng, Star wrote: > Reviewed-by: Star Zeng <star.z...@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:ler...@redhat.com] > Sent: Sunday, September 10, 2017 8:13 AM > To: edk2-devel-01 <edk2-devel@lists.01.org> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>; Dong, Eric > <eric.d...@intel.com>; Paulo Alcantara <pca...@zytor.com>; Ni, Ruiyu > <ruiyu...@intel.com>; Zeng, Star <star.z...@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.biesheu...@linaro.org> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Paulo Alcantara <pca...@zytor.com> > Cc: Ruiyu Ni <ruiyu...@intel.com> > Cc: Star Zeng <star.z...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek <ler...@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