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.ch...@hpe.com>
>
> Cc: Leif Lindholm <l...@nuviainc.com>
> Cc: Nickle Wang <nickle.w...@hpe.com>
> Cc: Michael D Kinney <michael.d.kin...@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?
Beyond that, this will probably be an issue for all architectures, so
why add explicit (identical) workarounds for IA32/X64?
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 (#70774): https://edk2.groups.io/g/devel/message/70774
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]
-=-=-=-=-=-=-=-=-=-=-=-
