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.lindh...@linaro.org> > Sent: 28 June 2018 05:51 PM > To: Sami Mujawar <sami.muja...@arm.com> > Cc: edk2-devel@lists.01.org; Evan Lloyd <evan.ll...@arm.com>; Matteo Carlini > <matteo.carl...@arm.com>; Stephanie Hughes-Fitt > <stephanie.hughes-f...@arm.com>; nd <n...@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.muja...@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
