From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by mx.groups.io with SMTP id smtpd.web08.6680.1611832626250790007 for ; Thu, 28 Jan 2021 03:17:06 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=FhY+GNY1; spf=pass (domain: nuviainc.com, ip: 209.85.128.42, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f42.google.com with SMTP id f16so3970136wmq.5 for ; Thu, 28 Jan 2021 03:17:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=y1gTlmRuBVc/maAF/62BWID6m5eLwCISUUc40hu1Zj0=; b=FhY+GNY1MVC68U0A6kqNqQc/VrPvUHhuUrOfY7Fx9UmjhTL37GoV8tWwEUZSsvc7pM 0N5WO/Rz7dYxMKoz7+szx4pH+01PxfBkqM3vBniCBaKLjB4SdzsVnyO/0rfOyGeraAt5 BjbUSqNlhQedXPXuY1/0UL8GkNxCKY5ijf6eCyU6Vho7lkgtGXotJ/RwsbAbbli0f5YF M/VC4M477PJxZ4cL8R7dpD9HkvHQHEo0YWrR9bNf3XYaHtmfwGD8G1MRVSKI21gB5hC+ kGMilEDPrTQ7TlVZSaWkllr5BhBfkk5D7bSU+XigCkPBlIy7m5GY6795BmnvjBrMBfpD 8Weg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=y1gTlmRuBVc/maAF/62BWID6m5eLwCISUUc40hu1Zj0=; b=C/BofR4hjbmSowOoJap2zTq76tOil8O+Coi1c9o+EuN2Rd9gWzsPgGB7dPq1GZmXKK nquA/q1M2/B9tF8B0+2EFQqTqfSvVBVesZWUu701UTAruSUVVPTAm1a97Fw9onPNv5ip jkWETIPtBEVh/wzUYXBKG6lcd1EMxeO8F1mbn6YUvgkAvTdfTB0ReKdZob5Fmql0G8JN aru6dS3TzxDxX2T7bFyoldiXoMrIt1+Qv5RqrPawmN4ew+4EgAIXJA3dh1h7xHZGg43v wXoSpeoPENhg9I8DfiwP2co7b4EIo2CDwQSeBKTZa6IYeCxCeiQ6cziyPD5it+vAS7rb ehEw== X-Gm-Message-State: AOAM532BmWuMEx3Ub3snkzHX3tBa+WnQy0iISwifF4uwsbKyxuMB0Z4T 5sqiYdFVfnLkpN2mzauV2E+SrQ== X-Google-Smtp-Source: ABdhPJz5KnfleIcPeFeM68WPZi6a8eIiks3P8u+emWYL23tIipr1+0HuSh1wgl/LR5ZT4+bWIOPbGg== X-Received: by 2002:a1c:a7c5:: with SMTP id q188mr8110847wme.108.1611832624756; Thu, 28 Jan 2021 03:17:04 -0800 (PST) Return-Path: Received: from vanye (cpc1-cmbg19-2-0-cust915.5-4.cable.virginm.net. [82.27.183.148]) by smtp.gmail.com with ESMTPSA id w4sm5565349wmc.13.2021.01.28.03.17.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Jan 2021 03:17:04 -0800 (PST) Date: Thu, 28 Jan 2021 11:17:02 +0000 From: "Leif Lindholm" To: "Chang, Abner (HPS SW/FW Technologist)" Cc: "devel@edk2.groups.io" , "Wang, Nickle (HPS SW)" , Michael D Kinney Subject: Re: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors Message-ID: <20210128111702.GT1664@vanye> References: <20210125043154.20645-1-abner.chang@hpe.com> <20210126113034.GO1664@vanye> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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? > > 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