Re: [edk2-devel] [PATCH 1/3] MdePkg/BaseLib: re-specify Base64Decode(), and add temporary stub impl
On 07/16/19 16:59, Philippe Mathieu-Daudé wrote: > On 7/16/19 4:14 PM, Laszlo Ersek wrote: >> On 07/16/19 11:41, Philippe Mathieu-Daudé wrote: >>> Hi Laszlo, >>> >>> On 7/16/19 10:38 AM, Philippe Mathieu-Daudé wrote: On 7/2/19 12:28 PM, Laszlo Ersek wrote: > Rewrite Base64Decode() from scratch, due to reasons listed in the second > reference below. > > As first step, redo the interface contract, and replace the current > implementation with a stub that asserts FALSE, then fails. > > Cc: Liming Gao > Cc: Marvin Häuser > Cc: Michael D Kinney > Cc: Philippe Mathieu-Daudé > Cc: Zhichao Gao > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1891 > Ref: > c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com">http://mid.mail-archive.com/c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com > Signed-off-by: Laszlo Ersek > --- > MdePkg/Include/Library/BaseLib.h | 99 +-- > MdePkg/Library/BaseLib/String.c | 285 ++-- > 2 files changed, 168 insertions(+), 216 deletions(-) >>> >>> You forgot to update the copyright in both files. >> >> I didn't: I never intended to. >> >> Updating or extending *existing* copyright notices is not a general edk2 >> requirement. Some companies insist that their associates do that, when >> they contribute patches. Red Hat doesn't (we don't extend copyright >> notices like that in QEMU either). > > Oh OK, I did not know :S In retrospect, I may have mislead you, unwittingly -- perhaps you recall me pointing out the same issue to other contributors. But, in those cases, I must have raised the topic only for one of two reasons: - I had known from experience that the contributor's employer would expect an update to the (C) notice - there was an update already in the patch, but it looked incorrect (extending a (C) notice to a year different from the current year, inadvertently deleting a different (C) notice, and so on). Thanks! Laszlo > Thanks for telling me, > > Phil. > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43815): https://edk2.groups.io/g/devel/message/43815 Mute This Topic: https://groups.io/mt/32284615/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/3] MdePkg/BaseLib: re-specify Base64Decode(), and add temporary stub impl
On 07/16/19 16:56, Gao, Liming wrote: > Laszlo: > Yes. Patch 1 & Patch 2 in MdePkg are both good to me. I have no other > comments. Reviewed-by: Liming Gao Many thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43809): https://edk2.groups.io/g/devel/message/43809 Mute This Topic: https://groups.io/mt/32284615/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/3] MdePkg/BaseLib: re-specify Base64Decode(), and add temporary stub impl
On 7/16/19 4:14 PM, Laszlo Ersek wrote: > On 07/16/19 11:41, Philippe Mathieu-Daudé wrote: >> Hi Laszlo, >> >> On 7/16/19 10:38 AM, Philippe Mathieu-Daudé wrote: >>> On 7/2/19 12:28 PM, Laszlo Ersek wrote: Rewrite Base64Decode() from scratch, due to reasons listed in the second reference below. As first step, redo the interface contract, and replace the current implementation with a stub that asserts FALSE, then fails. Cc: Liming Gao Cc: Marvin Häuser Cc: Michael D Kinney Cc: Philippe Mathieu-Daudé Cc: Zhichao Gao Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1891 Ref: c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com">http://mid.mail-archive.com/c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com Signed-off-by: Laszlo Ersek --- MdePkg/Include/Library/BaseLib.h | 99 +-- MdePkg/Library/BaseLib/String.c | 285 ++-- 2 files changed, 168 insertions(+), 216 deletions(-) >> >> You forgot to update the copyright in both files. > > I didn't: I never intended to. > > Updating or extending *existing* copyright notices is not a general edk2 > requirement. Some companies insist that their associates do that, when > they contribute patches. Red Hat doesn't (we don't extend copyright > notices like that in QEMU either). Oh OK, I did not know :S Thanks for telling me, Phil. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43795): https://edk2.groups.io/g/devel/message/43795 Mute This Topic: https://groups.io/mt/32284615/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/3] MdePkg/BaseLib: re-specify Base64Decode(), and add temporary stub impl
Laszlo:
Yes. Patch 1 & Patch 2 in MdePkg are both good to me. I have no other
comments. Reviewed-by: Liming Gao
Thanks
Liming
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo
> Ersek
> Sent: Tuesday, July 16, 2019 6:49 PM
> To: Gao, Liming ; Kinney, Michael D
>
> Cc: edk2-devel-groups-io ; Marvin Häuser
> ; Philippe Mathieu-Daudé
> ; Gao, Zhichao
> Subject: Re: [edk2-devel] [PATCH 1/3] MdePkg/BaseLib: re-specify
> Base64Decode(), and add temporary stub impl
>
> sending a separate ping here as well, for clarity -- Liming, Mike, can
> one of you please R-b or A-b this specific patch too?
>
> Thanks!
> Laszlo
>
> On 07/02/19 12:28, Laszlo Ersek wrote:
> > Rewrite Base64Decode() from scratch, due to reasons listed in the second
> > reference below.
> >
> > As first step, redo the interface contract, and replace the current
> > implementation with a stub that asserts FALSE, then fails.
> >
> > Cc: Liming Gao
> > Cc: Marvin Häuser
> > Cc: Michael D Kinney
> > Cc: Philippe Mathieu-Daudé
> > Cc: Zhichao Gao
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1891
> > Ref:
> > c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com">http://mid.mail-archive.com/c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com
> > Signed-off-by: Laszlo Ersek
> > ---
> > MdePkg/Include/Library/BaseLib.h | 99 +--
> > MdePkg/Library/BaseLib/String.c | 285 ++--
> > 2 files changed, 168 insertions(+), 216 deletions(-)
> >
> > diff --git a/MdePkg/Include/Library/BaseLib.h
> > b/MdePkg/Include/Library/BaseLib.h
> > index ebd7dd274cf4..5ef03e24edb1 100644
> > --- a/MdePkg/Include/Library/BaseLib.h
> > +++ b/MdePkg/Include/Library/BaseLib.h
> > @@ -2785,31 +2785,94 @@ Base64Encode (
> >);
> >
> > /**
> > - Convert Base64 ascii string to binary data based on RFC4648.
> > + Decode Base64 ASCII encoded data to 8-bit binary representation, based on
> > + RFC4648.
> >
> > - Produce Null-terminated binary data in the output buffer specified by
> > Destination and DestinationSize.
> > - The binary data is produced by converting the Base64 ascii string
> > specified by Source and SourceLength.
> > + Decoding occurs according to "Table 1: The Base 64 Alphabet" in RFC4648.
> >
> > - @param Source Input ASCII characters
> > - @param SourceLengthNumber of ASCII characters
> > - @param Destination Pointer to output buffer
> > - @param DestinationSize Caller is responsible for passing in buffer of at
> > least DestinationSize.
> > - Set 0 to get the size needed. Set to bytes stored
> > on return.
> > + Whitespace is ignored at all positions:
> > + - 0x09 ('\t') horizontal tab
> > + - 0x0A ('\n') new line
> > + - 0x0B ('\v') vertical tab
> > + - 0x0C ('\f') form feed
> > + - 0x0D ('\r') carriage return
> > + - 0x20 (' ') space
> >
> > - @retval RETURN_SUCCESS When binary buffer is filled in.
> > - @retval RETURN_INVALID_PARAMETER If Source is NULL or DestinationSize
> > is NULL.
> > - @retval RETURN_INVALID_PARAMETER If SourceLength or DestinationSize is
> > bigger than (MAX_ADDRESS -(UINTN)Destination ).
> > - @retval RETURN_INVALID_PARAMETER If there is any invalid character in
> > input stream.
> > - @retval RETURN_BUFFER_TOO_SMALLIf buffer length is smaller than
> > required buffer size.
> > + The minimum amount of required padding (with ASCII 0x3D, '=') is
> > tolerated
> > + and enforced at the end of the Base64 ASCII encoded data, and only there.
> >
> > - **/
> > + Other characters outside of the encoding alphabet cause the function to
> > + reject the Base64 ASCII encoded data.
> > +
> > + @param[in] Source Array of CHAR8 elements containing the
> > Base64
> > + ASCII encoding. May be NULL if
> > SourceSize is
> > + zero.
> > +
> > + @param[in] SourceSize Number of CHAR8 elements in Source.
> > +
> > + @param[out] Destination Array of UINT8 elements receiving the
> > decoded
> > + 8-bit binary representation. Allocated
> > by the
> > + caller. May be NULL if DestinationSize is
> > + zero on input. If NULL, decoding is
> > +
Re: [edk2-devel] [PATCH 1/3] MdePkg/BaseLib: re-specify Base64Decode(), and add temporary stub impl
On 07/16/19 11:41, Philippe Mathieu-Daudé wrote: > Hi Laszlo, > > On 7/16/19 10:38 AM, Philippe Mathieu-Daudé wrote: >> On 7/2/19 12:28 PM, Laszlo Ersek wrote: >>> Rewrite Base64Decode() from scratch, due to reasons listed in the second >>> reference below. >>> >>> As first step, redo the interface contract, and replace the current >>> implementation with a stub that asserts FALSE, then fails. >>> >>> Cc: Liming Gao >>> Cc: Marvin Häuser >>> Cc: Michael D Kinney >>> Cc: Philippe Mathieu-Daudé >>> Cc: Zhichao Gao >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1891 >>> Ref: >>> c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com">http://mid.mail-archive.com/c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com >>> Signed-off-by: Laszlo Ersek >>> --- >>> MdePkg/Include/Library/BaseLib.h | 99 +-- >>> MdePkg/Library/BaseLib/String.c | 285 ++-- >>> 2 files changed, 168 insertions(+), 216 deletions(-) > > You forgot to update the copyright in both files. I didn't: I never intended to. Updating or extending *existing* copyright notices is not a general edk2 requirement. Some companies insist that their associates do that, when they contribute patches. Red Hat doesn't (we don't extend copyright notices like that in QEMU either). Thanks, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43789): https://edk2.groups.io/g/devel/message/43789 Mute This Topic: https://groups.io/mt/32284615/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/3] MdePkg/BaseLib: re-specify Base64Decode(), and add temporary stub impl
sending a separate ping here as well, for clarity -- Liming, Mike, can
one of you please R-b or A-b this specific patch too?
Thanks!
Laszlo
On 07/02/19 12:28, Laszlo Ersek wrote:
> Rewrite Base64Decode() from scratch, due to reasons listed in the second
> reference below.
>
> As first step, redo the interface contract, and replace the current
> implementation with a stub that asserts FALSE, then fails.
>
> Cc: Liming Gao
> Cc: Marvin Häuser
> Cc: Michael D Kinney
> Cc: Philippe Mathieu-Daudé
> Cc: Zhichao Gao
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1891
> Ref:
> c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com">http://mid.mail-archive.com/c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com
> Signed-off-by: Laszlo Ersek
> ---
> MdePkg/Include/Library/BaseLib.h | 99 +--
> MdePkg/Library/BaseLib/String.c | 285 ++--
> 2 files changed, 168 insertions(+), 216 deletions(-)
>
> diff --git a/MdePkg/Include/Library/BaseLib.h
> b/MdePkg/Include/Library/BaseLib.h
> index ebd7dd274cf4..5ef03e24edb1 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -2785,31 +2785,94 @@ Base64Encode (
>);
>
> /**
> - Convert Base64 ascii string to binary data based on RFC4648.
> + Decode Base64 ASCII encoded data to 8-bit binary representation, based on
> + RFC4648.
>
> - Produce Null-terminated binary data in the output buffer specified by
> Destination and DestinationSize.
> - The binary data is produced by converting the Base64 ascii string
> specified by Source and SourceLength.
> + Decoding occurs according to "Table 1: The Base 64 Alphabet" in RFC4648.
>
> - @param Source Input ASCII characters
> - @param SourceLengthNumber of ASCII characters
> - @param Destination Pointer to output buffer
> - @param DestinationSize Caller is responsible for passing in buffer of at
> least DestinationSize.
> - Set 0 to get the size needed. Set to bytes stored
> on return.
> + Whitespace is ignored at all positions:
> + - 0x09 ('\t') horizontal tab
> + - 0x0A ('\n') new line
> + - 0x0B ('\v') vertical tab
> + - 0x0C ('\f') form feed
> + - 0x0D ('\r') carriage return
> + - 0x20 (' ') space
>
> - @retval RETURN_SUCCESS When binary buffer is filled in.
> - @retval RETURN_INVALID_PARAMETER If Source is NULL or DestinationSize is
> NULL.
> - @retval RETURN_INVALID_PARAMETER If SourceLength or DestinationSize is
> bigger than (MAX_ADDRESS -(UINTN)Destination ).
> - @retval RETURN_INVALID_PARAMETER If there is any invalid character in
> input stream.
> - @retval RETURN_BUFFER_TOO_SMALLIf buffer length is smaller than
> required buffer size.
> + The minimum amount of required padding (with ASCII 0x3D, '=') is tolerated
> + and enforced at the end of the Base64 ASCII encoded data, and only there.
>
> - **/
> + Other characters outside of the encoding alphabet cause the function to
> + reject the Base64 ASCII encoded data.
> +
> + @param[in] Source Array of CHAR8 elements containing the
> Base64
> + ASCII encoding. May be NULL if SourceSize
> is
> + zero.
> +
> + @param[in] SourceSize Number of CHAR8 elements in Source.
> +
> + @param[out] Destination Array of UINT8 elements receiving the
> decoded
> + 8-bit binary representation. Allocated by
> the
> + caller. May be NULL if DestinationSize is
> + zero on input. If NULL, decoding is
> + performed, but the 8-bit binary
> + representation is not stored. If non-NULL
> and
> + the function returns an error, the contents
> + of Destination are indeterminate.
> +
> + @param[in,out] DestinationSize On input, the number of UINT8 elements that
> + the caller allocated for Destination. On
> + output, if the function returns
> + RETURN_SUCCESS or RETURN_BUFFER_TOO_SMALL,
> + the number of UINT8 elements that are
> + required for decoding the Base64 ASCII
> + representation. If the function returns a
> + value different from both RETURN_SUCCESS
> and
> + RETURN_BUFFER_TOO_SMALL, then
> DestinationSize
> + is indeterminate on output.
> +
> + @retval RETURN_SUCCESSSourceSize CHAR8 elements at Source have
> +been decoded to on-output DestinationSize
> +UINT8 elements at Destination. Note that
> +
Re: [edk2-devel] [PATCH 1/3] MdePkg/BaseLib: re-specify Base64Decode(), and add temporary stub impl
Hi Laszlo, On 7/16/19 10:38 AM, Philippe Mathieu-Daudé wrote: > On 7/2/19 12:28 PM, Laszlo Ersek wrote: >> Rewrite Base64Decode() from scratch, due to reasons listed in the second >> reference below. >> >> As first step, redo the interface contract, and replace the current >> implementation with a stub that asserts FALSE, then fails. >> >> Cc: Liming Gao >> Cc: Marvin Häuser >> Cc: Michael D Kinney >> Cc: Philippe Mathieu-Daudé >> Cc: Zhichao Gao >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1891 >> Ref: >> c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com">http://mid.mail-archive.com/c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com >> Signed-off-by: Laszlo Ersek >> --- >> MdePkg/Include/Library/BaseLib.h | 99 +-- >> MdePkg/Library/BaseLib/String.c | 285 ++-- >> 2 files changed, 168 insertions(+), 216 deletions(-) You forgot to update the copyright in both files. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43776): https://edk2.groups.io/g/devel/message/43776 Mute This Topic: https://groups.io/mt/32284615/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/3] MdePkg/BaseLib: re-specify Base64Decode(), and add temporary stub impl
On 7/2/19 12:28 PM, Laszlo Ersek wrote:
> Rewrite Base64Decode() from scratch, due to reasons listed in the second
> reference below.
>
> As first step, redo the interface contract, and replace the current
> implementation with a stub that asserts FALSE, then fails.
>
> Cc: Liming Gao
> Cc: Marvin Häuser
> Cc: Michael D Kinney
> Cc: Philippe Mathieu-Daudé
> Cc: Zhichao Gao
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1891
> Ref:
> c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com">http://mid.mail-archive.com/c495bd0b-ea4d-7206-8a4f-a7149760d19a@redhat.com
> Signed-off-by: Laszlo Ersek
> ---
> MdePkg/Include/Library/BaseLib.h | 99 +--
> MdePkg/Library/BaseLib/String.c | 285 ++--
> 2 files changed, 168 insertions(+), 216 deletions(-)
>
> diff --git a/MdePkg/Include/Library/BaseLib.h
> b/MdePkg/Include/Library/BaseLib.h
> index ebd7dd274cf4..5ef03e24edb1 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -2785,31 +2785,94 @@ Base64Encode (
>);
>
> /**
> - Convert Base64 ascii string to binary data based on RFC4648.
> + Decode Base64 ASCII encoded data to 8-bit binary representation, based on
> + RFC4648.
>
> - Produce Null-terminated binary data in the output buffer specified by
> Destination and DestinationSize.
> - The binary data is produced by converting the Base64 ascii string
> specified by Source and SourceLength.
> + Decoding occurs according to "Table 1: The Base 64 Alphabet" in RFC4648.
>
> - @param Source Input ASCII characters
> - @param SourceLengthNumber of ASCII characters
> - @param Destination Pointer to output buffer
> - @param DestinationSize Caller is responsible for passing in buffer of at
> least DestinationSize.
> - Set 0 to get the size needed. Set to bytes stored
> on return.
> + Whitespace is ignored at all positions:
> + - 0x09 ('\t') horizontal tab
> + - 0x0A ('\n') new line
> + - 0x0B ('\v') vertical tab
> + - 0x0C ('\f') form feed
> + - 0x0D ('\r') carriage return
> + - 0x20 (' ') space
>
> - @retval RETURN_SUCCESS When binary buffer is filled in.
> - @retval RETURN_INVALID_PARAMETER If Source is NULL or DestinationSize is
> NULL.
> - @retval RETURN_INVALID_PARAMETER If SourceLength or DestinationSize is
> bigger than (MAX_ADDRESS -(UINTN)Destination ).
> - @retval RETURN_INVALID_PARAMETER If there is any invalid character in
> input stream.
> - @retval RETURN_BUFFER_TOO_SMALLIf buffer length is smaller than
> required buffer size.
> + The minimum amount of required padding (with ASCII 0x3D, '=') is tolerated
> + and enforced at the end of the Base64 ASCII encoded data, and only there.
>
> - **/
> + Other characters outside of the encoding alphabet cause the function to
> + reject the Base64 ASCII encoded data.
> +
> + @param[in] Source Array of CHAR8 elements containing the
> Base64
> + ASCII encoding. May be NULL if SourceSize
> is
> + zero.
> +
> + @param[in] SourceSize Number of CHAR8 elements in Source.
> +
> + @param[out] Destination Array of UINT8 elements receiving the
> decoded
> + 8-bit binary representation. Allocated by
> the
> + caller. May be NULL if DestinationSize is
> + zero on input. If NULL, decoding is
> + performed, but the 8-bit binary
> + representation is not stored. If non-NULL
> and
> + the function returns an error, the contents
> + of Destination are indeterminate.
> +
> + @param[in,out] DestinationSize On input, the number of UINT8 elements that
> + the caller allocated for Destination. On
> + output, if the function returns
> + RETURN_SUCCESS or RETURN_BUFFER_TOO_SMALL,
> + the number of UINT8 elements that are
> + required for decoding the Base64 ASCII
> + representation. If the function returns a
> + value different from both RETURN_SUCCESS
> and
> + RETURN_BUFFER_TOO_SMALL, then
> DestinationSize
> + is indeterminate on output.
> +
> + @retval RETURN_SUCCESSSourceSize CHAR8 elements at Source have
> +been decoded to on-output DestinationSize
> +UINT8 elements at Destination. Note that
> +RETURN_SUCCESS covers the case when
> +DestinationSize is zero on input, and
> +
