| Message ID | 20161021212737.15974-19-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 21 October 2016 at 22:27, Laszlo Ersek <lersek@redhat.com> wrote: > AsciiStrCat() is deprecated / disabled under the > DISABLE_NEW_DEPRECATED_INTERFACES feature test macro. > > The caller of CpsrString() is required to pass in "ReturnStr" with 32 > CHAR8 elements. (DefaultExceptionHandler() complies with this.) "Str" is > used to build "ReturnStr" gradually. Just before calling AsciiStrCat(), > "Str" points to the then-terminating NUL character in "ReturnStr". > > The difference (Str - ReturnStr) gives the number of non-NUL characters > we've written thus far, hence (32 - (Str - ReturnStr)) yields the number > of remaining bytes in ReturnStr, including the ultimately terminating NUL > character. > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Michael Zimmermann <sigmaepsilon92@gmail.com> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=164 > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=165 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > > Notes: > - build-tested only > > - Michael (CC'd) had posted a patch earlier for this: > <https://lists.01.org/pipermail/edk2-devel/2016-August/000489.html>, > but I only noticed that now that he pointed it out, in > <https://lists.01.org/pipermail/edk2-devel/2016-October/003121.html>. > I'll leave it to the ArmPkg maintainers to choose one; I don't feel > strongly either way. > > ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c > index aece26355e2e..4b2ee9a9acf7 100644 > --- a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c > +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c > @@ -116,7 +116,10 @@ CpsrString ( > break; > } > > - AsciiStrCat (Str, ModeStr); > + // > + // See the interface contract in the leading comment block. > + // > + AsciiStrCatS (Str, 32 - (Str - ReturnStr), ModeStr); > return; Could you please use a symbolic constant for this '32', and replace it in the declaration of CpsrStr[] as well? With that Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Oh, and if the bogus 'return' happens to get lost along the way as well, I would not mind. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 10/24/16 09:57, Ard Biesheuvel wrote: > On 21 October 2016 at 22:27, Laszlo Ersek <lersek@redhat.com> wrote: >> AsciiStrCat() is deprecated / disabled under the >> DISABLE_NEW_DEPRECATED_INTERFACES feature test macro. >> >> The caller of CpsrString() is required to pass in "ReturnStr" with 32 >> CHAR8 elements. (DefaultExceptionHandler() complies with this.) "Str" is >> used to build "ReturnStr" gradually. Just before calling AsciiStrCat(), >> "Str" points to the then-terminating NUL character in "ReturnStr". >> >> The difference (Str - ReturnStr) gives the number of non-NUL characters >> we've written thus far, hence (32 - (Str - ReturnStr)) yields the number >> of remaining bytes in ReturnStr, including the ultimately terminating NUL >> character. >> >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Cc: Leif Lindholm <leif.lindholm@linaro.org> >> Cc: Michael Zimmermann <sigmaepsilon92@gmail.com> >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=164 >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=165 >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> >> Notes: >> - build-tested only >> >> - Michael (CC'd) had posted a patch earlier for this: >> <https://lists.01.org/pipermail/edk2-devel/2016-August/000489.html>, >> but I only noticed that now that he pointed it out, in >> <https://lists.01.org/pipermail/edk2-devel/2016-October/003121.html>. >> I'll leave it to the ArmPkg maintainers to choose one; I don't feel >> strongly either way. >> >> ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c >> index aece26355e2e..4b2ee9a9acf7 100644 >> --- a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c >> +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c >> @@ -116,7 +116,10 @@ CpsrString ( >> break; >> } >> >> - AsciiStrCat (Str, ModeStr); >> + // >> + // See the interface contract in the leading comment block. >> + // >> + AsciiStrCatS (Str, 32 - (Str - ReturnStr), ModeStr); >> return; > > Could you please use a symbolic constant for this '32', and replace it > in the declaration of CpsrStr[] as well? > > With that > > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Oh, and if the bogus 'return' happens to get lost along the way as > well, I would not mind. > Okay, will do. :) _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c index aece26355e2e..4b2ee9a9acf7 100644 --- a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c @@ -116,7 +116,10 @@ CpsrString ( break; } - AsciiStrCat (Str, ModeStr); + // + // See the interface contract in the leading comment block. + // + AsciiStrCatS (Str, 32 - (Str - ReturnStr), ModeStr); return; }
AsciiStrCat() is deprecated / disabled under the DISABLE_NEW_DEPRECATED_INTERFACES feature test macro. The caller of CpsrString() is required to pass in "ReturnStr" with 32 CHAR8 elements. (DefaultExceptionHandler() complies with this.) "Str" is used to build "ReturnStr" gradually. Just before calling AsciiStrCat(), "Str" points to the then-terminating NUL character in "ReturnStr". The difference (Str - ReturnStr) gives the number of non-NUL characters we've written thus far, hence (32 - (Str - ReturnStr)) yields the number of remaining bytes in ReturnStr, including the ultimately terminating NUL character. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Leif Lindholm <leif.lindholm@linaro.org> Cc: Michael Zimmermann <sigmaepsilon92@gmail.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=164 Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=165 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: - build-tested only - Michael (CC'd) had posted a patch earlier for this: <https://lists.01.org/pipermail/edk2-devel/2016-August/000489.html>, but I only noticed that now that he pointed it out, in <https://lists.01.org/pipermail/edk2-devel/2016-October/003121.html>. I'll leave it to the ArmPkg maintainers to choose one; I don't feel strongly either way. ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) -- 2.9.2 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel