[PATCH for-4.21 1/6] tools/{lib,}xl: fix usage of error return from json_tokener_parse_verbose()

Roger Pau Monne posted 6 patches 2 weeks, 1 day ago
[PATCH for-4.21 1/6] tools/{lib,}xl: fix usage of error return from json_tokener_parse_verbose()
Posted by Roger Pau Monne 2 weeks, 1 day ago
It's possible for json_tokener_parse_verbose() to return NULL and leave the
error parameter unset.  Initialize the error token to success, and only
print it if the function has actually set it to a value different than
success.

Reported by XenServer internal Coverity instance.

Fixes: 7e95dab9eb63 ("libxl: Convert libxl__json_parse() to use json-c")
Fixes: f6c6f2679d49 ("libxl: Convert libxl__object_to_json() to json-c")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 tools/libs/light/libxl_json.c | 6 ++++--
 tools/xl/xl_info.c            | 5 +++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/libs/light/libxl_json.c b/tools/libs/light/libxl_json.c
index c76ae9f64a9d..a9e06b06932d 100644
--- a/tools/libs/light/libxl_json.c
+++ b/tools/libs/light/libxl_json.c
@@ -1366,11 +1366,13 @@ libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s)
     libxl__json_object *o = NULL;
 #ifdef USE_LIBJSONC_PARSER
     json_object *jso;
-    enum json_tokener_error error;
+    enum json_tokener_error error = json_tokener_success;
 
     jso = json_tokener_parse_verbose(s, &error);
     if (!jso) {
-        LOG(ERROR, "json-c parse error: %s", json_tokener_error_desc(error));
+        LOG(ERROR, "json-c parse error: %s",
+            error != json_tokener_success ? json_tokener_error_desc(error)
+                                          : "unspecified error");
         goto out;
     }
 #endif
diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
index 80a3b25aac81..777ff2c64294 100644
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -67,7 +67,7 @@ static int printf_info_one_json(json_object **jso_r, int domid,
 {
     json_object *jso = NULL;
     json_object *jso_config = NULL;
-    enum json_tokener_error error;
+    enum json_tokener_error error = json_tokener_success;
     char *s = NULL;
     int r = EXIT_FAILURE;
 
@@ -75,7 +75,8 @@ static int printf_info_one_json(json_object **jso_r, int domid,
     jso_config = json_tokener_parse_verbose(s, &error);
     if (!jso_config) {
         fprintf(stderr, "fail to parse JSON from libxl_domain_config_to_json(): %s\n",
-                json_tokener_error_desc(error));
+                error != json_tokener_success ? json_tokener_error_desc(error)
+                                              : "unspecified error");
         goto out;
     }
 
-- 
2.51.0


Re: [PATCH for-4.21 1/6] tools/{lib,}xl: fix usage of error return from json_tokener_parse_verbose()
Posted by Andrew Cooper 2 weeks, 1 day ago
On 15/10/2025 2:40 pm, Roger Pau Monne wrote:
> diff --git a/tools/libs/light/libxl_json.c b/tools/libs/light/libxl_json.c
> index c76ae9f64a9d..a9e06b06932d 100644
> --- a/tools/libs/light/libxl_json.c
> +++ b/tools/libs/light/libxl_json.c
> @@ -1366,11 +1366,13 @@ libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s)
>      libxl__json_object *o = NULL;
>  #ifdef USE_LIBJSONC_PARSER
>      json_object *jso;
> -    enum json_tokener_error error;
> +    enum json_tokener_error error = json_tokener_success;

Looking at the options available, I'd suggest initialising to:

    json_tokener_error_parse_unexpected

and dropping the rest of the hunk.  I wouldn't assume that success
cannot be passed here.

~Andrew

>  
>      jso = json_tokener_parse_verbose(s, &error);
>      if (!jso) {
> -        LOG(ERROR, "json-c parse error: %s", json_tokener_error_desc(error));
> +        LOG(ERROR, "json-c parse error: %s",
> +            error != json_tokener_success ? json_tokener_error_desc(error)
> +                                          : "unspecified error");
>          goto out;
>      }
>  #endif


Re: [PATCH for-4.21 1/6] tools/{lib,}xl: fix usage of error return from json_tokener_parse_verbose()
Posted by Roger Pau Monné 2 weeks, 1 day ago
On Wed, Oct 15, 2025 at 02:59:25PM +0100, Andrew Cooper wrote:
> On 15/10/2025 2:40 pm, Roger Pau Monne wrote:
> > diff --git a/tools/libs/light/libxl_json.c b/tools/libs/light/libxl_json.c
> > index c76ae9f64a9d..a9e06b06932d 100644
> > --- a/tools/libs/light/libxl_json.c
> > +++ b/tools/libs/light/libxl_json.c
> > @@ -1366,11 +1366,13 @@ libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s)
> >      libxl__json_object *o = NULL;
> >  #ifdef USE_LIBJSONC_PARSER
> >      json_object *jso;
> > -    enum json_tokener_error error;
> > +    enum json_tokener_error error = json_tokener_success;
> 
> Looking at the options available, I'd suggest initialising to:
> 
>     json_tokener_error_parse_unexpected
> 
> and dropping the rest of the hunk.  I wouldn't assume that success
> cannot be passed here.

That error code translates to "unexpected character", which I didn't
think was very accurate here.  I didn't find any good error code to
map here, hence why I went with this kind of weird solution.

I don't mind using json_tokener_error_parse_unexpected, just
mentioning why I didn't use it in the first place.  Anthony, what's
your opinion?

Thanks, Roger.