* [PATCH] RedfishPkg/JsonLib: Fix build errors
@ 2021-01-25 4:31 Abner Chang
2021-01-26 11:30 ` [edk2-devel] " Leif Lindholm
2021-02-18 3:29 ` Nickle Wang
0 siblings, 2 replies; 9+ messages in thread
From: Abner Chang @ 2021-01-25 4:31 UTC (permalink / raw)
To: devel; +Cc: Leif Lindholm, Nickle Wang, Michael D Kinney
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@hpe.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Nickle Wang <nickle.wang@hpe.com>
Cc: Michael D Kinney <michael.d.kinney@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
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
2021-01-25 4:31 [PATCH] RedfishPkg/JsonLib: Fix build errors Abner Chang
@ 2021-01-26 11:30 ` Leif Lindholm
2021-01-28 4:02 ` Abner Chang
2021-02-18 3:29 ` Nickle Wang
1 sibling, 1 reply; 9+ messages in thread
From: Leif Lindholm @ 2021-01-26 11:30 UTC (permalink / raw)
To: devel, abner.chang; +Cc: Nickle Wang, Michael D Kinney
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@hpe.com>
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Nickle Wang <nickle.wang@hpe.com>
> Cc: Michael D Kinney <michael.d.kinney@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
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
2021-01-26 11:30 ` [edk2-devel] " Leif Lindholm
@ 2021-01-28 4:02 ` Abner Chang
2021-01-28 11:17 ` Leif Lindholm
0 siblings, 1 reply; 9+ messages in thread
From: Abner Chang @ 2021-01-28 4:02 UTC (permalink / raw)
To: devel@edk2.groups.io, leif@nuviainc.com
Cc: Wang, Nickle (HPS SW), Michael D Kinney
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Leif Lindholm
> Sent: Tuesday, January 26, 2021 7:31 PM
> To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> <abner.chang@hpe.com>
> Cc: Wang, Nickle (HPS SW) <nickle.wang@hpe.com>; Michael D Kinney
> <michael.d.kinney@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@hpe.com>
> >
> > Cc: Leif Lindholm <leif@nuviainc.com>
> > Cc: Nickle Wang <nickle.wang@hpe.com>
> > Cc: Michael D Kinney <michael.d.kinney@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
> >
> >
> >
> >
> >
> >
>
>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
2021-01-28 4:02 ` Abner Chang
@ 2021-01-28 11:17 ` Leif Lindholm
2021-01-28 14:30 ` Abner Chang
0 siblings, 1 reply; 9+ messages in thread
From: Leif Lindholm @ 2021-01-28 11:17 UTC (permalink / raw)
To: Chang, Abner (HPS SW/FW Technologist)
Cc: devel@edk2.groups.io, Wang, Nickle (HPS SW), Michael D Kinney
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
2021-01-28 11:17 ` Leif Lindholm
@ 2021-01-28 14:30 ` Abner Chang
2021-01-29 5:11 ` Abner Chang
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Abner Chang @ 2021-01-28 14:30 UTC (permalink / raw)
To: Leif Lindholm
Cc: devel@edk2.groups.io, Wang, Nickle (HPS SW), Michael D Kinney
> -----Original Message-----
> From: Leif Lindholm [mailto:leif@nuviainc.com]
> Sent: Thursday, January 28, 2021 7:17 PM
> To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>
> Cc: devel@edk2.groups.io; Wang, Nickle (HPS SW) <nickle.wang@hpe.com>;
> Michael D Kinney <michael.d.kinney@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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
2021-01-28 14:30 ` Abner Chang
@ 2021-01-29 5:11 ` Abner Chang
2021-02-08 5:05 ` Abner Chang
[not found] ` <CS1PR8401MB1144811ED28E1602035E6D7DFFB99@CS1PR8401MB1144.NAMPRD84.PROD.OUTLOOK.COM>
2 siblings, 0 replies; 9+ messages in thread
From: Abner Chang @ 2021-01-29 5:11 UTC (permalink / raw)
To: Leif Lindholm
Cc: devel@edk2.groups.io, Wang, Nickle (HPS SW), Michael D Kinney
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 <leif@nuviainc.com>
> Cc: devel@edk2.groups.io; Wang, Nickle (HPS SW) <nickle.wang@hpe.com>;
> Michael D Kinney <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
>
>
>
> > -----Original Message-----
> > From: Leif Lindholm [mailto:leif@nuviainc.com]
> > Sent: Thursday, January 28, 2021 7:17 PM
> > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>
> > Cc: devel@edk2.groups.io; Wang, Nickle (HPS SW)
> <nickle.wang@hpe.com>;
> > Michael D Kinney <michael.d.kinney@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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
2021-01-28 14:30 ` Abner Chang
2021-01-29 5:11 ` Abner Chang
@ 2021-02-08 5:05 ` Abner Chang
[not found] ` <CS1PR8401MB1144811ED28E1602035E6D7DFFB99@CS1PR8401MB1144.NAMPRD84.PROD.OUTLOOK.COM>
2 siblings, 0 replies; 9+ messages in thread
From: Abner Chang @ 2021-02-08 5:05 UTC (permalink / raw)
To: Leif Lindholm
Cc: devel@edk2.groups.io, Wang, Nickle (HPS SW), Michael D Kinney
> -----Original Message-----
> From: Chang, Abner (HPS SW/FW Technologist)
> Sent: Thursday, January 28, 2021 10:31 PM
> To: Leif Lindholm <leif@nuviainc.com>
> Cc: devel@edk2.groups.io; Wang, Nickle (HPS SW) <nickle.wang@hpe.com>;
> Michael D Kinney <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
>
>
>
> > -----Original Message-----
> > From: Leif Lindholm [mailto:leif@nuviainc.com]
> > Sent: Thursday, January 28, 2021 7:17 PM
> > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>
> > Cc: devel@edk2.groups.io; Wang, Nickle (HPS SW)
> <nickle.wang@hpe.com>;
> > Michael D Kinney <michael.d.kinney@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?
I am working on the PR of your patch for jannson community, however I realize that the patch doesn't work well with the jansson lib. The purpose of this macro is to define the for-loop statement, but the "break" in the code body breaks the usage of these macros. Below is one of the use cases of macro "json_object_foreach" which is used as the for-loop statement and simple to use for users to write the code block,
In value.c,
json_object_foreach(other, key, value) {
json_t *v = json_object_get(object, key);
if (json_is_object(v) && json_is_object(value)) {
if (do_object_update_recursive(v, value, parents)) {
res = -1;
break;
}
} else {
if (json_object_set_nocheck(object, key, value)) {
res = -1;
break;
}
}
}
I would just keep these macros and the source code which uses these macros unchanged. Use the build option to ignore the warning message on VS for the open source jansson lib.
Regards,
Abner
>
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
[not found] ` <CS1PR8401MB1144811ED28E1602035E6D7DFFB99@CS1PR8401MB1144.NAMPRD84.PROD.OUTLOOK.COM>
@ 2021-02-18 3:27 ` Abner Chang
0 siblings, 0 replies; 9+ messages in thread
From: Abner Chang @ 2021-02-18 3:27 UTC (permalink / raw)
To: Leif Lindholm
Cc: devel@edk2.groups.io, Wang, Nickle (HPS SW), Michael D Kinney
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 <leif@nuviainc.com>
> Cc: devel@edk2.groups.io; Wang, Nickle (HPS SW) <nickle.wang@hpe.com>;
> Michael D Kinney <michael.d.kinney@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 <leif@nuviainc.com>
> > Cc: devel@edk2.groups.io; Wang, Nickle (HPS SW)
> <nickle.wang@hpe.com>;
> > Michael D Kinney <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors
> >
> >
> >
> > > -----Original Message-----
> > > From: Leif Lindholm [mailto:leif@nuviainc.com]
> > > Sent: Thursday, January 28, 2021 7:17 PM
> > > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>
> > > Cc: devel@edk2.groups.io; Wang, Nickle (HPS SW)
> > <nickle.wang@hpe.com>;
> > > Michael D Kinney <michael.d.kinney@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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] RedfishPkg/JsonLib: Fix build errors
2021-01-25 4:31 [PATCH] RedfishPkg/JsonLib: Fix build errors Abner Chang
2021-01-26 11:30 ` [edk2-devel] " Leif Lindholm
@ 2021-02-18 3:29 ` Nickle Wang
1 sibling, 0 replies; 9+ messages in thread
From: Nickle Wang @ 2021-02-18 3:29 UTC (permalink / raw)
To: Chang, Abner (HPS SW/FW Technologist), devel@edk2.groups.io
Cc: Leif Lindholm, Michael D Kinney
Hi Abner,
You many want to update the function description. It mentioned "UTF8 encoded string" but the input string type is CHAR8, not CHAR16.
Thanks,
Nickle
> -----Original Message-----
> From: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>
> Sent: Monday, January 25, 2021 12:32 PM
> To: devel@edk2.groups.io
> Cc: Leif Lindholm <leif@nuviainc.com>; Wang, Nickle (HPS SW)
> <nickle.wang@hpe.com>; Michael D Kinney <michael.d.kinney@intel.com>
> Subject: [PATCH] RedfishPkg/JsonLib: Fix build errors
>
> 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@hpe.com>
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Nickle Wang <nickle.wang@hpe.com>
> Cc: Michael D Kinney <michael.d.kinney@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
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-02-18 3:29 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-25 4:31 [PATCH] RedfishPkg/JsonLib: Fix build errors Abner Chang
2021-01-26 11:30 ` [edk2-devel] " Leif Lindholm
2021-01-28 4:02 ` Abner Chang
2021-01-28 11:17 ` Leif Lindholm
2021-01-28 14:30 ` Abner Chang
2021-01-29 5:11 ` Abner Chang
2021-02-08 5:05 ` Abner Chang
[not found] ` <CS1PR8401MB1144811ED28E1602035E6D7DFFB99@CS1PR8401MB1144.NAMPRD84.PROD.OUTLOOK.COM>
2021-02-18 3:27 ` Abner Chang
2021-02-18 3:29 ` Nickle Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox