The ALIGN32 macro had undesired side effects when used with binary
operators and was generating compilation errors.
Added brackets to fix this issue.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Notes:
v1:
- Fix macro side effect [SAMI]
DynamicTablesPkg/Include/Library/TableHelperLib.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/DynamicTablesPkg/Include/Library/TableHelperLib.h b/DynamicTablesPkg/Include/Library/TableHelperLib.h
index b358223434af76820d34c29c67325919a2283aa7..3eac28a158e9621f6f33a5b538964a16e6b69112 100644
--- a/DynamicTablesPkg/Include/Library/TableHelperLib.h
+++ b/DynamicTablesPkg/Include/Library/TableHelperLib.h
@@ -17,7 +17,7 @@
/** A helper macro to align a value to the 32-bit word boundary
*/
-#define ALIGN32(x) ((x) + (sizeof (UINT32) - 1)) & ~(sizeof (UINT32) - 1)
+#define ALIGN32(x) (((x) + (sizeof (UINT32) - 1)) & ~(sizeof (UINT32) - 1))
/** The GetCgfMgrInfo function gets the CM_STD_OBJ_CONFIGURATION_MANAGER_INFO
object from the Configuration Manager.
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Jun 27, 2018 at 05:47:45PM +0100, Sami Mujawar wrote:
> The ALIGN32 macro had undesired side effects when used with binary
> operators and was generating compilation errors.
>
> Added brackets to fix this issue.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>
> Notes:
> v1:
> - Fix macro side effect [SAMI]
>
> DynamicTablesPkg/Include/Library/TableHelperLib.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/DynamicTablesPkg/Include/Library/TableHelperLib.h b/DynamicTablesPkg/Include/Library/TableHelperLib.h
> index b358223434af76820d34c29c67325919a2283aa7..3eac28a158e9621f6f33a5b538964a16e6b69112 100644
> --- a/DynamicTablesPkg/Include/Library/TableHelperLib.h
> +++ b/DynamicTablesPkg/Include/Library/TableHelperLib.h
> @@ -17,7 +17,7 @@
>
> /** A helper macro to align a value to the 32-bit word boundary
> */
> -#define ALIGN32(x) ((x) + (sizeof (UINT32) - 1)) & ~(sizeof (UINT32) - 1)
> +#define ALIGN32(x) (((x) + (sizeof (UINT32) - 1)) & ~(sizeof (UINT32) - 1))
Not as such a comment on this patch, but this problem could also be
resolved by dropping this local macro in favour of the Base.h
ALIGN_VALUE(x, 4) ... or just redefining it to use that.
/
Leif
>
> /** The GetCgfMgrInfo function gets the CM_STD_OBJ_CONFIGURATION_MANAGER_INFO
> object from the Configuration Manager.
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Hi Leif,
I agree it will be good to update the code to use ALIGN_VALUE(x,4).
I can see 2 options to fix this:
- Resubmit a v2 patchset with this change.
OR
- I can submit a separate patch (on top of the current patchset) for this change.
Can you let me know which one is preferred, please?
Regards,
Sami Mujawar
-----Original Message-----
From: Leif Lindholm <leif.lindholm@linaro.org>
Sent: 28 June 2018 05:51 PM
To: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: edk2-devel@lists.01.org; Evan Lloyd <Evan.Lloyd@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd <nd@arm.com>
Subject: Re: [staging/dynamictables PATCH v1 4/5] DynamicTablesPkg: Fix macro to prevent side effect
On Wed, Jun 27, 2018 at 05:47:45PM +0100, Sami Mujawar wrote:
> The ALIGN32 macro had undesired side effects when used with binary
> operators and was generating compilation errors.
>
> Added brackets to fix this issue.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>
> Notes:
> v1:
> - Fix macro side effect [SAMI]
>
> DynamicTablesPkg/Include/Library/TableHelperLib.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/DynamicTablesPkg/Include/Library/TableHelperLib.h
> b/DynamicTablesPkg/Include/Library/TableHelperLib.h
> index
> b358223434af76820d34c29c67325919a2283aa7..3eac28a158e9621f6f33a5b53896
> 4a16e6b69112 100644
> --- a/DynamicTablesPkg/Include/Library/TableHelperLib.h
> +++ b/DynamicTablesPkg/Include/Library/TableHelperLib.h
> @@ -17,7 +17,7 @@
>
> /** A helper macro to align a value to the 32-bit word boundary */
> -#define ALIGN32(x) ((x) + (sizeof (UINT32) - 1)) & ~(sizeof (UINT32)
> - 1)
> +#define ALIGN32(x) (((x) + (sizeof (UINT32) - 1)) & ~(sizeof (UINT32)
> +- 1))
Not as such a comment on this patch, but this problem could also be resolved by dropping this local macro in favour of the Base.h ALIGN_VALUE(x, 4) ... or just redefining it to use that.
/
Leif
>
> /** The GetCgfMgrInfo function gets the CM_STD_OBJ_CONFIGURATION_MANAGER_INFO
> object from the Configuration Manager.
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On Thu, Jun 28, 2018 at 05:10:48PM +0000, Sami Mujawar wrote: > Hi Leif, > > I agree it will be good to update the code to use ALIGN_VALUE(x,4). > > I can see 2 options to fix this: > - Resubmit a v2 patchset with this change. > OR > - I can submit a separate patch (on top of the current patchset) for this change. > > Can you let me know which one is preferred, please? It won't actually garble the history to do it as a separate patch, so I'm happy with that. But I'm still waiting on a clarification from Evan. Best Regards, Leif > Regards, > > Sami Mujawar > -----Original Message----- > From: Leif Lindholm <leif.lindholm@linaro.org> > Sent: 28 June 2018 05:51 PM > To: Sami Mujawar <Sami.Mujawar@arm.com> > Cc: edk2-devel@lists.01.org; Evan Lloyd <Evan.Lloyd@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd <nd@arm.com> > Subject: Re: [staging/dynamictables PATCH v1 4/5] DynamicTablesPkg: Fix macro to prevent side effect > > On Wed, Jun 27, 2018 at 05:47:45PM +0100, Sami Mujawar wrote: > > The ALIGN32 macro had undesired side effects when used with binary > > operators and was generating compilation errors. > > > > Added brackets to fix this issue. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com> > > --- > > > > Notes: > > v1: > > - Fix macro side effect [SAMI] > > > > DynamicTablesPkg/Include/Library/TableHelperLib.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/DynamicTablesPkg/Include/Library/TableHelperLib.h > > b/DynamicTablesPkg/Include/Library/TableHelperLib.h > > index > > b358223434af76820d34c29c67325919a2283aa7..3eac28a158e9621f6f33a5b53896 > > 4a16e6b69112 100644 > > --- a/DynamicTablesPkg/Include/Library/TableHelperLib.h > > +++ b/DynamicTablesPkg/Include/Library/TableHelperLib.h > > @@ -17,7 +17,7 @@ > > > > /** A helper macro to align a value to the 32-bit word boundary */ > > -#define ALIGN32(x) ((x) + (sizeof (UINT32) - 1)) & ~(sizeof (UINT32) > > - 1) > > +#define ALIGN32(x) (((x) + (sizeof (UINT32) - 1)) & ~(sizeof (UINT32) > > +- 1)) > > Not as such a comment on this patch, but this problem could also be resolved by dropping this local macro in favour of the Base.h ALIGN_VALUE(x, 4) ... or just redefining it to use that. > > / > Leif > > > > > /** The GetCgfMgrInfo function gets the CM_STD_OBJ_CONFIGURATION_MANAGER_INFO > > object from the Configuration Manager. > > -- > > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' > > > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2026 Red Hat, Inc.