| Message ID | 1477651721-16958-2-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 5211ece936eedb0a29ea170b449e0af8edff8db2 |
| Headers | show |
On 10/28/16 12:48, Ard Biesheuvel wrote: > Get rid of functions that are no longer available when defining > DISABLE_NEW_DEPRECATED_INTERFACES > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > index 4d0811cc5eaf..64b25f8a8c45 100644 > --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > @@ -269,7 +269,7 @@ ArmFastbootPlatformInit ( > > // Copy handle and partition name > Entry->PartitionHandle = AllHandles[LoopIndex]; > - StrnCpy ( > + CopyMem ( > Entry->PartitionName, > PartitionEntries[PartitionNode->PartitionNumber - 1].PartitionName, // Partition numbers start from 1. > PARTITION_NAME_MAX_LENGTH > @@ -320,7 +320,8 @@ ArmFastbootPlatformFlashPartition ( > CHAR16 PartitionNameUnicode[60]; > BOOLEAN PartitionFound; > > - AsciiStrToUnicodeStr (PartitionName, PartitionNameUnicode); > + AsciiStrToUnicodeStrS (PartitionName, PartitionNameUnicode, > + ARRAY_SIZE (PartitionNameUnicode)); Congratulations on the first genuine use of the now-central ARRAY_SIZE() macro! :) > > PartitionFound = FALSE; > Entry = (FASTBOOT_PARTITION_LIST *) GetFirstNode (&(mPartitionListHead)); > @@ -396,7 +397,7 @@ ArmFastbootPlatformGetVar ( > ) > { > if (AsciiStrCmp (Name, "product")) { > - AsciiStrCpy (Value, FixedPcdGetPtr (PcdFirmwareVendor)); > + AsciiStrCpyS (Value, 61, FixedPcdGetPtr (PcdFirmwareVendor)); > } else { > *Value = '\0'; > } Right. > @@ -410,7 +411,7 @@ ArmFastbootPlatformOemCommand ( > { > CHAR16 CommandUnicode[65]; > > - AsciiStrToUnicodeStr (Command, CommandUnicode); > + AsciiStrToUnicodeStrS (Command, CommandUnicode, ARRAY_SIZE (CommandUnicode)); > > if (AsciiStrCmp (Command, "Demonstrate") == 0) { > DEBUG ((EFI_D_ERROR, "ARM OEM Fastboot command 'Demonstrate' received.\n")); > I knew ARRAY_SIZE() would be especially useful with these "safe string" functions! :) Reviewed-by: Laszlo Ersek <lersek@redhat.com> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Fri, Oct 28, 2016 at 11:48:40AM +0100, Ard Biesheuvel wrote: > Get rid of functions that are no longer available when defining > DISABLE_NEW_DEPRECATED_INTERFACES > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > index 4d0811cc5eaf..64b25f8a8c45 100644 > --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > @@ -269,7 +269,7 @@ ArmFastbootPlatformInit ( > > // Copy handle and partition name > Entry->PartitionHandle = AllHandles[LoopIndex]; > - StrnCpy ( > + CopyMem ( > Entry->PartitionName, > PartitionEntries[PartitionNode->PartitionNumber - 1].PartitionName, // Partition numbers start from 1. > PARTITION_NAME_MAX_LENGTH > @@ -320,7 +320,8 @@ ArmFastbootPlatformFlashPartition ( > CHAR16 PartitionNameUnicode[60]; > BOOLEAN PartitionFound; > > - AsciiStrToUnicodeStr (PartitionName, PartitionNameUnicode); > + AsciiStrToUnicodeStrS (PartitionName, PartitionNameUnicode, > + ARRAY_SIZE (PartitionNameUnicode)); > > PartitionFound = FALSE; > Entry = (FASTBOOT_PARTITION_LIST *) GetFirstNode (&(mPartitionListHead)); > @@ -396,7 +397,7 @@ ArmFastbootPlatformGetVar ( > ) > { > if (AsciiStrCmp (Name, "product")) { > - AsciiStrCpy (Value, FixedPcdGetPtr (PcdFirmwareVendor)); > + AsciiStrCpyS (Value, 61, FixedPcdGetPtr (PcdFirmwareVendor)); I'm totally OK with the reason for hard-coding this, but could you add the comment from the feedback on previous version?: // FASTBOOT_COMMAND_MAX_LENGTH - 4 + NULL terminator (Or if there's a better way of putting it.) > } else { > *Value = '\0'; > } > @@ -410,7 +411,7 @@ ArmFastbootPlatformOemCommand ( > { > CHAR16 CommandUnicode[65]; > > - AsciiStrToUnicodeStr (Command, CommandUnicode); > + AsciiStrToUnicodeStrS (Command, CommandUnicode, ARRAY_SIZE (CommandUnicode)); > > if (AsciiStrCmp (Command, "Demonstrate") == 0) { > DEBUG ((EFI_D_ERROR, "ARM OEM Fastboot command 'Demonstrate' received.\n")); > -- > 2.7.4 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 28 October 2016 at 15:48, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Fri, Oct 28, 2016 at 11:48:40AM +0100, Ard Biesheuvel wrote: >> Get rid of functions that are no longer available when defining >> DISABLE_NEW_DEPRECATED_INTERFACES >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >> index 4d0811cc5eaf..64b25f8a8c45 100644 >> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >> @@ -269,7 +269,7 @@ ArmFastbootPlatformInit ( >> >> // Copy handle and partition name >> Entry->PartitionHandle = AllHandles[LoopIndex]; >> - StrnCpy ( >> + CopyMem ( >> Entry->PartitionName, >> PartitionEntries[PartitionNode->PartitionNumber - 1].PartitionName, // Partition numbers start from 1. >> PARTITION_NAME_MAX_LENGTH >> @@ -320,7 +320,8 @@ ArmFastbootPlatformFlashPartition ( >> CHAR16 PartitionNameUnicode[60]; >> BOOLEAN PartitionFound; >> >> - AsciiStrToUnicodeStr (PartitionName, PartitionNameUnicode); >> + AsciiStrToUnicodeStrS (PartitionName, PartitionNameUnicode, >> + ARRAY_SIZE (PartitionNameUnicode)); >> >> PartitionFound = FALSE; >> Entry = (FASTBOOT_PARTITION_LIST *) GetFirstNode (&(mPartitionListHead)); >> @@ -396,7 +397,7 @@ ArmFastbootPlatformGetVar ( >> ) >> { >> if (AsciiStrCmp (Name, "product")) { >> - AsciiStrCpy (Value, FixedPcdGetPtr (PcdFirmwareVendor)); >> + AsciiStrCpyS (Value, 61, FixedPcdGetPtr (PcdFirmwareVendor)); > > I'm totally OK with the reason for hard-coding this, but could you add > the comment from the feedback on previous version?: > // FASTBOOT_COMMAND_MAX_LENGTH - 4 + NULL terminator > (Or if there's a better way of putting it.) > What if I decorate each entry point with the comment block from the associated protocol header file instead? (in a follow up patch) That is common practice in Tianocore anyway, and looks like this for this particular protocol method: /* If the variable referred to by Name exists, copy it (as a null-terminated string) into Value. If it doesn't exist, put the Empty string in Value. Variable names and values may not be larger than 60 bytes, excluding the terminal null character. This is a limitation of the Fastboot protocol. The Fastboot application will handle platform-nonspecific variables (Currently "version" is the only one of these.) @param[in] Name Null-terminated name of Fastboot variable to retrieve. @param[out] Value Caller-allocated buffer for null-terminated value of variable. @retval EFI_SUCCESS The variable was retrieved, or it doesn't exist. @retval EFI_DEVICE_ERROR There was an error looking up the variable. This does _not_ include the variable not existing. */ _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 10/28/16 17:02, Ard Biesheuvel wrote: > On 28 October 2016 at 15:48, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> On Fri, Oct 28, 2016 at 11:48:40AM +0100, Ard Biesheuvel wrote: >>> Get rid of functions that are no longer available when defining >>> DISABLE_NEW_DEPRECATED_INTERFACES >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> --- >>> ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c | 9 +++++---- >>> 1 file changed, 5 insertions(+), 4 deletions(-) >>> >>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >>> index 4d0811cc5eaf..64b25f8a8c45 100644 >>> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >>> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >>> @@ -269,7 +269,7 @@ ArmFastbootPlatformInit ( >>> >>> // Copy handle and partition name >>> Entry->PartitionHandle = AllHandles[LoopIndex]; >>> - StrnCpy ( >>> + CopyMem ( >>> Entry->PartitionName, >>> PartitionEntries[PartitionNode->PartitionNumber - 1].PartitionName, // Partition numbers start from 1. >>> PARTITION_NAME_MAX_LENGTH >>> @@ -320,7 +320,8 @@ ArmFastbootPlatformFlashPartition ( >>> CHAR16 PartitionNameUnicode[60]; >>> BOOLEAN PartitionFound; >>> >>> - AsciiStrToUnicodeStr (PartitionName, PartitionNameUnicode); >>> + AsciiStrToUnicodeStrS (PartitionName, PartitionNameUnicode, >>> + ARRAY_SIZE (PartitionNameUnicode)); >>> >>> PartitionFound = FALSE; >>> Entry = (FASTBOOT_PARTITION_LIST *) GetFirstNode (&(mPartitionListHead)); >>> @@ -396,7 +397,7 @@ ArmFastbootPlatformGetVar ( >>> ) >>> { >>> if (AsciiStrCmp (Name, "product")) { >>> - AsciiStrCpy (Value, FixedPcdGetPtr (PcdFirmwareVendor)); >>> + AsciiStrCpyS (Value, 61, FixedPcdGetPtr (PcdFirmwareVendor)); >> >> I'm totally OK with the reason for hard-coding this, but could you add >> the comment from the feedback on previous version?: >> // FASTBOOT_COMMAND_MAX_LENGTH - 4 + NULL terminator >> (Or if there's a better way of putting it.) >> > > What if I decorate each entry point with the comment block from the > associated protocol header file instead? (in a follow up patch) > That is common practice in Tianocore anyway, Indeed it is. Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Fri, Oct 28, 2016 at 04:02:21PM +0100, Ard Biesheuvel wrote: > On 28 October 2016 at 15:48, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Fri, Oct 28, 2016 at 11:48:40AM +0100, Ard Biesheuvel wrote: > >> Get rid of functions that are no longer available when defining > >> DISABLE_NEW_DEPRECATED_INTERFACES > >> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> --- > >> ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c | 9 +++++---- > >> 1 file changed, 5 insertions(+), 4 deletions(-) > >> > >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > >> index 4d0811cc5eaf..64b25f8a8c45 100644 > >> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > >> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > >> @@ -269,7 +269,7 @@ ArmFastbootPlatformInit ( > >> > >> // Copy handle and partition name > >> Entry->PartitionHandle = AllHandles[LoopIndex]; > >> - StrnCpy ( > >> + CopyMem ( > >> Entry->PartitionName, > >> PartitionEntries[PartitionNode->PartitionNumber - 1].PartitionName, // Partition numbers start from 1. > >> PARTITION_NAME_MAX_LENGTH > >> @@ -320,7 +320,8 @@ ArmFastbootPlatformFlashPartition ( > >> CHAR16 PartitionNameUnicode[60]; > >> BOOLEAN PartitionFound; > >> > >> - AsciiStrToUnicodeStr (PartitionName, PartitionNameUnicode); > >> + AsciiStrToUnicodeStrS (PartitionName, PartitionNameUnicode, > >> + ARRAY_SIZE (PartitionNameUnicode)); > >> > >> PartitionFound = FALSE; > >> Entry = (FASTBOOT_PARTITION_LIST *) GetFirstNode (&(mPartitionListHead)); > >> @@ -396,7 +397,7 @@ ArmFastbootPlatformGetVar ( > >> ) > >> { > >> if (AsciiStrCmp (Name, "product")) { > >> - AsciiStrCpy (Value, FixedPcdGetPtr (PcdFirmwareVendor)); > >> + AsciiStrCpyS (Value, 61, FixedPcdGetPtr (PcdFirmwareVendor)); > > > > I'm totally OK with the reason for hard-coding this, but could you add > > the comment from the feedback on previous version?: > > // FASTBOOT_COMMAND_MAX_LENGTH - 4 + NULL terminator > > (Or if there's a better way of putting it.) > > > > What if I decorate each entry point with the comment block from the > associated protocol header file instead? (in a follow up patch) > That is common practice in Tianocore anyway, and looks like this for > this particular protocol method: > > /* > If the variable referred to by Name exists, copy it (as a null-terminated > string) into Value. If it doesn't exist, put the Empty string in Value. > > Variable names and values may not be larger than 60 bytes, excluding the > terminal null character. This is a limitation of the Fastboot protocol. > > The Fastboot application will handle platform-nonspecific variables > (Currently "version" is the only one of these.) > > @param[in] Name Null-terminated name of Fastboot variable to retrieve. > @param[out] Value Caller-allocated buffer for null-terminated value of > variable. > > @retval EFI_SUCCESS The variable was retrieved, or it doesn't exist. > @retval EFI_DEVICE_ERROR There was an error looking up the variable. This > does _not_ include the variable not existing. > */ That works for me. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 28 October 2016 at 16:13, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Fri, Oct 28, 2016 at 04:02:21PM +0100, Ard Biesheuvel wrote: >> On 28 October 2016 at 15:48, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> > On Fri, Oct 28, 2016 at 11:48:40AM +0100, Ard Biesheuvel wrote: >> >> Get rid of functions that are no longer available when defining >> >> DISABLE_NEW_DEPRECATED_INTERFACES >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> --- >> >> ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c | 9 +++++---- >> >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> >> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >> >> index 4d0811cc5eaf..64b25f8a8c45 100644 >> >> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >> >> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >> >> @@ -269,7 +269,7 @@ ArmFastbootPlatformInit ( >> >> >> >> // Copy handle and partition name >> >> Entry->PartitionHandle = AllHandles[LoopIndex]; >> >> - StrnCpy ( >> >> + CopyMem ( >> >> Entry->PartitionName, >> >> PartitionEntries[PartitionNode->PartitionNumber - 1].PartitionName, // Partition numbers start from 1. >> >> PARTITION_NAME_MAX_LENGTH >> >> @@ -320,7 +320,8 @@ ArmFastbootPlatformFlashPartition ( >> >> CHAR16 PartitionNameUnicode[60]; >> >> BOOLEAN PartitionFound; >> >> >> >> - AsciiStrToUnicodeStr (PartitionName, PartitionNameUnicode); >> >> + AsciiStrToUnicodeStrS (PartitionName, PartitionNameUnicode, >> >> + ARRAY_SIZE (PartitionNameUnicode)); >> >> >> >> PartitionFound = FALSE; >> >> Entry = (FASTBOOT_PARTITION_LIST *) GetFirstNode (&(mPartitionListHead)); >> >> @@ -396,7 +397,7 @@ ArmFastbootPlatformGetVar ( >> >> ) >> >> { >> >> if (AsciiStrCmp (Name, "product")) { >> >> - AsciiStrCpy (Value, FixedPcdGetPtr (PcdFirmwareVendor)); >> >> + AsciiStrCpyS (Value, 61, FixedPcdGetPtr (PcdFirmwareVendor)); >> > >> > I'm totally OK with the reason for hard-coding this, but could you add >> > the comment from the feedback on previous version?: >> > // FASTBOOT_COMMAND_MAX_LENGTH - 4 + NULL terminator >> > (Or if there's a better way of putting it.) >> > >> >> What if I decorate each entry point with the comment block from the >> associated protocol header file instead? (in a follow up patch) >> That is common practice in Tianocore anyway, and looks like this for >> this particular protocol method: >> >> /* >> If the variable referred to by Name exists, copy it (as a null-terminated >> string) into Value. If it doesn't exist, put the Empty string in Value. >> >> Variable names and values may not be larger than 60 bytes, excluding the >> terminal null character. This is a limitation of the Fastboot protocol. >> >> The Fastboot application will handle platform-nonspecific variables >> (Currently "version" is the only one of these.) >> >> @param[in] Name Null-terminated name of Fastboot variable to retrieve. >> @param[out] Value Caller-allocated buffer for null-terminated value of >> variable. >> >> @retval EFI_SUCCESS The variable was retrieved, or it doesn't exist. >> @retval EFI_DEVICE_ERROR There was an error looking up the variable. This >> does _not_ include the variable not existing. >> */ > > That works for me. > Good. Does that mean you are happy with this patch as-is then? _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Fri, Oct 28, 2016 at 04:14:43PM +0100, Ard Biesheuvel wrote: > On 28 October 2016 at 16:13, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Fri, Oct 28, 2016 at 04:02:21PM +0100, Ard Biesheuvel wrote: > >> On 28 October 2016 at 15:48, Leif Lindholm <leif.lindholm@linaro.org> wrote: > >> > On Fri, Oct 28, 2016 at 11:48:40AM +0100, Ard Biesheuvel wrote: > >> >> Get rid of functions that are no longer available when defining > >> >> DISABLE_NEW_DEPRECATED_INTERFACES > >> >> > >> >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> >> --- > >> >> ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c | 9 +++++---- > >> >> 1 file changed, 5 insertions(+), 4 deletions(-) > >> >> > >> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > >> >> index 4d0811cc5eaf..64b25f8a8c45 100644 > >> >> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > >> >> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > >> >> @@ -269,7 +269,7 @@ ArmFastbootPlatformInit ( > >> >> > >> >> // Copy handle and partition name > >> >> Entry->PartitionHandle = AllHandles[LoopIndex]; > >> >> - StrnCpy ( > >> >> + CopyMem ( > >> >> Entry->PartitionName, > >> >> PartitionEntries[PartitionNode->PartitionNumber - 1].PartitionName, // Partition numbers start from 1. > >> >> PARTITION_NAME_MAX_LENGTH > >> >> @@ -320,7 +320,8 @@ ArmFastbootPlatformFlashPartition ( > >> >> CHAR16 PartitionNameUnicode[60]; > >> >> BOOLEAN PartitionFound; > >> >> > >> >> - AsciiStrToUnicodeStr (PartitionName, PartitionNameUnicode); > >> >> + AsciiStrToUnicodeStrS (PartitionName, PartitionNameUnicode, > >> >> + ARRAY_SIZE (PartitionNameUnicode)); > >> >> > >> >> PartitionFound = FALSE; > >> >> Entry = (FASTBOOT_PARTITION_LIST *) GetFirstNode (&(mPartitionListHead)); > >> >> @@ -396,7 +397,7 @@ ArmFastbootPlatformGetVar ( > >> >> ) > >> >> { > >> >> if (AsciiStrCmp (Name, "product")) { > >> >> - AsciiStrCpy (Value, FixedPcdGetPtr (PcdFirmwareVendor)); > >> >> + AsciiStrCpyS (Value, 61, FixedPcdGetPtr (PcdFirmwareVendor)); > >> > > >> > I'm totally OK with the reason for hard-coding this, but could you add > >> > the comment from the feedback on previous version?: > >> > // FASTBOOT_COMMAND_MAX_LENGTH - 4 + NULL terminator > >> > (Or if there's a better way of putting it.) > >> > > >> > >> What if I decorate each entry point with the comment block from the > >> associated protocol header file instead? (in a follow up patch) > >> That is common practice in Tianocore anyway, and looks like this for > >> this particular protocol method: > >> > >> /* > >> If the variable referred to by Name exists, copy it (as a null-terminated > >> string) into Value. If it doesn't exist, put the Empty string in Value. > >> > >> Variable names and values may not be larger than 60 bytes, excluding the > >> terminal null character. This is a limitation of the Fastboot protocol. > >> > >> The Fastboot application will handle platform-nonspecific variables > >> (Currently "version" is the only one of these.) > >> > >> @param[in] Name Null-terminated name of Fastboot variable to retrieve. > >> @param[out] Value Caller-allocated buffer for null-terminated value of > >> variable. > >> > >> @retval EFI_SUCCESS The variable was retrieved, or it doesn't exist. > >> @retval EFI_DEVICE_ERROR There was an error looking up the variable. This > >> does _not_ include the variable not existing. > >> */ > > > > That works for me. > > > > Good. Does that mean you are happy with this patch as-is then? If it goes after the one I just acked, sure: Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c index 4d0811cc5eaf..64b25f8a8c45 100644 --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c @@ -269,7 +269,7 @@ ArmFastbootPlatformInit ( // Copy handle and partition name Entry->PartitionHandle = AllHandles[LoopIndex]; - StrnCpy ( + CopyMem ( Entry->PartitionName, PartitionEntries[PartitionNode->PartitionNumber - 1].PartitionName, // Partition numbers start from 1. PARTITION_NAME_MAX_LENGTH @@ -320,7 +320,8 @@ ArmFastbootPlatformFlashPartition ( CHAR16 PartitionNameUnicode[60]; BOOLEAN PartitionFound; - AsciiStrToUnicodeStr (PartitionName, PartitionNameUnicode); + AsciiStrToUnicodeStrS (PartitionName, PartitionNameUnicode, + ARRAY_SIZE (PartitionNameUnicode)); PartitionFound = FALSE; Entry = (FASTBOOT_PARTITION_LIST *) GetFirstNode (&(mPartitionListHead)); @@ -396,7 +397,7 @@ ArmFastbootPlatformGetVar ( ) { if (AsciiStrCmp (Name, "product")) { - AsciiStrCpy (Value, FixedPcdGetPtr (PcdFirmwareVendor)); + AsciiStrCpyS (Value, 61, FixedPcdGetPtr (PcdFirmwareVendor)); } else { *Value = '\0'; } @@ -410,7 +411,7 @@ ArmFastbootPlatformOemCommand ( { CHAR16 CommandUnicode[65]; - AsciiStrToUnicodeStr (Command, CommandUnicode); + AsciiStrToUnicodeStrS (Command, CommandUnicode, ARRAY_SIZE (CommandUnicode)); if (AsciiStrCmp (Command, "Demonstrate") == 0) { DEBUG ((EFI_D_ERROR, "ARM OEM Fastboot command 'Demonstrate' received.\n"));
Get rid of functions that are no longer available when defining DISABLE_NEW_DEPRECATED_INTERFACES Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel