Hi Leif, Would you like to recheck the new patches I sent? Thanks Abner
> -----Original Message----- > From: Chang, Abner (HPS SW/FW Technologist) > Sent: Friday, January 29, 2021 1:11 PM > To: Leif Lindholm <l...@nuviainc.com> > Cc: devel@edk2.groups.io; Wang, Nickle (HPS SW) <nickle.w...@hpe.com>; > Michael D Kinney <michael.d.kin...@intel.com> > Subject: RE: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors > > Hi Leif, > I split this patch into 3 separate patches. Please ignore the old one. > > Thank > Abner > > > -----Original Message----- > > From: Chang, Abner (HPS SW/FW Technologist) > > Sent: Thursday, January 28, 2021 10:31 PM > > To: Leif Lindholm <l...@nuviainc.com> > > Cc: devel@edk2.groups.io; Wang, Nickle (HPS SW) > <nickle.w...@hpe.com>; > > Michael D Kinney <michael.d.kin...@intel.com> > > Subject: RE: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors > > > > > > > > > -----Original Message----- > > > From: Leif Lindholm [mailto:l...@nuviainc.com] > > > Sent: Thursday, January 28, 2021 7:17 PM > > > To: Chang, Abner (HPS SW/FW Technologist) <abner.ch...@hpe.com> > > > Cc: devel@edk2.groups.io; Wang, Nickle (HPS SW) > > <nickle.w...@hpe.com>; > > > Michael D Kinney <michael.d.kin...@intel.com> > > > Subject: Re: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build > > > errors > > > > > > On Thu, Jan 28, 2021 at 04:02:54 +0000, Chang, Abner (HPS SW/FW > > > Technologist) wrote: > > > > > > + # C4706: assignment within conditional expression > > > > > > # > > > > > > # Define macro HAVE_CONFIG_H to include > > > > > > jansson_private_config.h to > > > > > build. > > > > > > # Undefined _WIN32, WIN64, _MSC_VER macros > > > > > > # On GCC, no error on the unused-function and > > > > > > unused-but-set- > > > variable. > > > > > > # > > > > > > - MSFT:*_*_X64_CC_FLAGS = /wd4204 /wd4244 /wd4090 /wd4334 > > > > > > /DHAVE_CONFIG_H=1 /U_WIN32 /UWIN64 /U_MSC_VER > > > > > > - MSFT:*_*_IA32_CC_FLAGS = /wd4204 /wd4244 /wd4090 > > > > > /DHAVE_CONFIG_H=1 > > > > > > /U_WIN32 /UWIN64 /U_MSC_VER > > > > > > + MSFT:*_*_X64_CC_FLAGS = /wd4204 /wd4244 /wd4090 /wd4334 > > > > > /wd4706 > > > > > > + /DHAVE_CONFIG_H=1 /U_WIN32 /UWIN64 /U_MSC_VER > > > > > > + MSFT:*_*_IA32_CC_FLAGS = /wd4204 /wd4244 /wd4090 /wd4706 > > > > > > + /DHAVE_CONFIG_H=1 /U_WIN32 /UWIN64 /U_MSC_VER > > > > > > > > > > Urgh, please don't do this. > > > > > C4706 is what gives you a warning for accidentally assigning > > > > > instead of comparing. It's our last defence against the > > > > > jeopardy-comparing hordes that think > > > > > if (NULL == Ptr) > > > > > is a sensible way of writing C. > > > > > > > > > > What is the actual problem being encountered? > > > > > > > > That is because we use the macro defined in open source header > > > > file, RedfishPkg/Library/JsonLib/jansson/src/jansson.h > > > > > > > > #define json_object_foreach(object, key, value) > > > > \ > > > > for (key = json_object_iter_key(json_object_iter(object)); > > \ > > > > key && (value = > > > json_object_iter_value(json_object_key_to_iter(key))); \ > > > > key = json_object_iter_key( > > > > \ > > > > json_object_iter_next(object, > > > > json_object_key_to_iter(key)))) > > > > > > Yay, "optimisation" by using preprocessor... > > > > > > Apart from how this ought to be a helper function: > > > - No parentheses around parameters in macro. > > > - Setting "value" at each iteration through the loop condition. > > > - Slinging parentheses like it's a lisp convention. > > > - And it would be worse if they treated the parameters properly. > > > > > > If we ignore the other issues, the below should be functionally > > > equivalent and build on VS without disabling the warning: > > > > > > for (key = json_object_iter_key(json_object_iter(object)); > \ > > > key; > > > \ > > > key = json_object_iter_key( > > > \ > > > json_object_iter_next(object, json_object_key_to_iter(key)))) > > > { \ > > > value = json_object_iter_value(json_object_key_to_iter(key)); > > > \ > > > if (!value) > > > \ > > > break; > > > \ > > > } > > > > > > Could you try to convince upstream to take the patch? > > Sure, I just sent an email to community, hope we can get the feedback > soon. > > > > Leif, that may takes time to have this patch to be in upstream (if > > they agree with it)... I have another PR which is not get merged yet. > > This will block our works on edk2, thus I would like to just add build > > option to suppress this build error. The build option is only for > > JsonLib though. Do you agree with this? > > > > Thanks > > Abner > > > > > > > > > > Beyond that, this will probably be an issue for all > > > > > architectures, so why add explicit (identical) workarounds for > IA32/X64? > > > > > > > > I didn't catch this build error on GCC. You may know why? > > > > > > Ugh. > > > This is because (for whatever reason), both clang and GCC suppress > > > this warning if you add parentheses around the assignment. VS does not. > > > > > > / > > > Leif -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#71767): https://edk2.groups.io/g/devel/message/71767 Mute This Topic: https://groups.io/mt/80097450/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
