[edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
Abner Chang
abner.chang at hpe.com
Thu Jan 28 04:02:54 UTC 2021
> -----Original Message-----
> From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of
> Leif Lindholm
> Sent: Tuesday, January 26, 2021 7:31 PM
> To: devel at edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> <abner.chang at hpe.com>
> Cc: Wang, Nickle (HPS SW) <nickle.wang at hpe.com>; Michael D Kinney
> <michael.d.kinney at intel.com>
> Subject: Re: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
>
> Hi Abner,
>
> On Mon, Jan 25, 2021 at 12:31:54 +0800, Abner Chang wrote:
> > This patch fixes the build errors when build JsonLib with EDK2 Redfish
> > feature driver.
> >
> > - Add JsonLoadString function to load a NULL terminated-string JSON
> > - json_string_value() in JsonValueGetAsciiString () is removed by
> > accident.
> >
> > Signed-off-by: Abner Chang <abner.chang at hpe.com>
> >
> > Cc: Leif Lindholm <leif at nuviainc.com>
> > Cc: Nickle Wang <nickle.wang at hpe.com>
> > Cc: Michael D Kinney <michael.d.kinney at intel.com>
> > ---
> > RedfishPkg/Library/JsonLib/JsonLib.inf | 5 +++--
> > RedfishPkg/Include/Library/JsonLib.h | 21 ++++++++++++++++++
> > RedfishPkg/Library/JsonLib/JsonLib.c | 30
> ++++++++++++++++++++++++--
> > 3 files changed, 52 insertions(+), 4 deletions(-)
> >
> > diff --git a/RedfishPkg/Library/JsonLib/JsonLib.inf
> > b/RedfishPkg/Library/JsonLib/JsonLib.inf
> > index 48b094a78a..9d52a622e1 100644
> > --- a/RedfishPkg/Library/JsonLib/JsonLib.inf
> > +++ b/RedfishPkg/Library/JsonLib/JsonLib.inf
> > @@ -75,12 +75,13 @@
> > # C4244: conversion from type1 to type2, possible loss of data
> > # C4334: 32-bit shift implicitly converted to 64-bit
> > # C4204: nonstandard extension used: non-constant aggregate initializer
> > + # 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))))
>
> 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?
Abner
>
> Secondly, I understand these changes were added for a single reason "fix
> build failures" - but these are two distinct changes, so should be two distinct
> patches.
>
> /
> Leif
>
> > GCC:*_*_*_CC_FLAGS = -Wno-unused-function
> > -Wno-unused-but-set-variable
> >
> > diff --git a/RedfishPkg/Include/Library/JsonLib.h
> > b/RedfishPkg/Include/Library/JsonLib.h
> > index 3c10f67d27..82ca4bad60 100644
> > --- a/RedfishPkg/Include/Library/JsonLib.h
> > +++ b/RedfishPkg/Include/Library/JsonLib.h
> > @@ -664,6 +664,27 @@ JsonDumpString (
> > IN UINTN Flags
> > );
> >
> > +/**
> > + Convert a string to JSON object.
> > + The function is used to convert a NULL terminated UTF8 encoded
> > +string to a JSON
> > + value. Only object and array represented strings can be converted
> > +successfully,
> > + since they are the only valid root values of a JSON text for UEFI usage.
> > +
> > + Real number and number with exponent part are not supportted by UEFI.
> > +
> > + Caller needs to cleanup the root value by calling JsonValueFree().
> > +
> > + @param[in] String The NULL terminated UTF8 encoded string to
> convert
> > +
> > + @retval Array JSON value or object JSON value, or NULL when any
> error occurs.
> > +
> > +**/
> > +EDKII_JSON_VALUE
> > +EFIAPI
> > +JsonLoadString (
> > + IN CONST CHAR8* String
> > + );
> > +
> > /**
> > Load JSON from a buffer.
> >
> > diff --git a/RedfishPkg/Library/JsonLib/JsonLib.c
> > b/RedfishPkg/Library/JsonLib/JsonLib.c
> > index 34ff381aee..1762c6f5af 100644
> > --- a/RedfishPkg/Library/JsonLib/JsonLib.c
> > +++ b/RedfishPkg/Library/JsonLib/JsonLib.c
> > @@ -430,10 +430,10 @@ JsonValueGetAsciiString (
> > IN EDKII_JSON_VALUE Json
> > )
> > {
> > - CHAR8 *AsciiStr;
> > + CONST CHAR8 *AsciiStr;
> > UINTN Index;
> >
> > - AsciiStr = (CHAR8 *) ((json_t *) Json);
> > + AsciiStr = json_string_value ((json_t *) Json);
> > if (AsciiStr == NULL) {
> > return NULL;
> > }
> > @@ -819,6 +819,32 @@ JsonDumpString (
> > return json_dumps((json_t *)JsonValue, Flags); }
> >
> > +/**
> > + Convert a string to JSON object.
> > + The function is used to convert a NULL terminated UTF8 encoded
> > +string to a JSON
> > + value. Only object and array represented strings can be converted
> > +successfully,
> > + since they are the only valid root values of a JSON text for UEFI usage.
> > +
> > + Real number and number with exponent part are not supportted by UEFI.
> > +
> > + Caller needs to cleanup the root value by calling JsonValueFree().
> > +
> > + @param[in] String The NULL terminated UTF8 encoded string to
> convert
> > +
> > + @retval Array JSON value or object JSON value, or NULL when any
> error occurs.
> > +
> > +**/
> > +EDKII_JSON_VALUE
> > +EFIAPI
> > +JsonLoadString (
> > + IN CONST CHAR8* String
> > + )
> > +{
> > + json_error_t JsonError;
> > +
> > + return (EDKII_JSON_VALUE) json_loads ((const char *)String, 0,
> > +&JsonError); }
> > +
> > /**
> > Load JSON from a buffer.
> >
> > --
> > 2.17.1
> >
> >
> >
> >
> >
> >
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70839): https://edk2.groups.io/g/devel/message/70839
Mute This Topic: https://groups.io/mt/80097450/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list