All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony PERARD <anthony@xenproject.org>
To: Jason Andryuk <jason.andryuk@amd.com>
Cc: xen-devel@lists.xenproject.org,
	Anthony PERARD <anthony.perard@vates.tech>,
	Juergen Gross <jgross@suse.com>
Subject: Re: [XEN PATCH 06/11] libxl: convert libxl__json_object_to_yajl_gen to libxl__json_object_to_libjsonc_object
Date: Fri, 29 Aug 2025 15:56:03 +0200	[thread overview]
Message-ID: <aLGxc2d5rZspn9wj@l14> (raw)
In-Reply-To: <10a60455-a4d2-4c58-8a80-d8b264d27efd@amd.com>

On Wed, Aug 27, 2025 at 11:37:07AM -0400, Jason Andryuk wrote:
> On 2025-08-08 10:55, Anthony PERARD wrote:
> > +    case JSON_NUMBER:
> > +        *jso_out = json_object_new_string(obj->u.string);
> 
> Is JSON_NUMBER calling json_object_new_string() correct?  It looks like the
> yajl code falls back to a string, so that is okay but surprising.

Yeah, I think that's correct.
:-( maybe not. Even if we have these too comments:

    In libxl_internal.h, enum libxl__json_node_type:
        /* number is store in string, it's too big to be a long long or a double */
        JSON_NUMBER  = (1 << 4),

    In json_callback_number():
        /* If the conversion fail, we just store the original string. */

With yajl, we call yajl_gen_number(), which probably write 2^128 as:

    340282366920938463463374607431768211456

but this new json-c generator would write instead:

    "340282366920938463463374607431768211456"

I guess we might be able to replicate the same behavior by using
json_object_set_serializer() or json_object_new_double_s() (which use
the former). But I don't know if it is worth the effort. I hope we won't
have int bigger than 64 bits.

And there's probably no tests for JSON_NUMBERs. So I guess first step
would be to write a test that have numbers that can't be converted to
`long long` and see what happens.

Thanks,

-- 
Anthony PERARD


  reply	other threads:[~2025-08-29 13:56 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-08 14:55 [XEN PATCH 00/11] Allow to build libxl and other tools with json-c instead of yajl Anthony PERARD
2025-08-08 14:55 ` [XEN PATCH 01/11] xl: move printf_info prototype to an header Anthony PERARD
2025-08-27 14:55   ` Jason Andryuk
2025-08-08 14:55 ` [XEN PATCH 02/11] libxl: Remove duplicate libxl_domain_config_gen_json prototype Anthony PERARD
2025-08-27 14:55   ` Jason Andryuk
2025-08-08 14:55 ` [XEN PATCH 03/11] libxl: remove duplicated libxl__yajl_gen_asciiz() prototype Anthony PERARD
2025-08-27 14:56   ` Jason Andryuk
2025-08-08 14:55 ` [XEN PATCH 04/11] tools/configure: Introduce deps on json-c lib for libxl Anthony PERARD
2025-08-11 10:48   ` Andrew Cooper
2025-08-13 15:42     ` Anthony PERARD
2025-08-27 15:01   ` Jason Andryuk
2025-08-29 13:17     ` Anthony PERARD
2025-08-31 14:42       ` Jason Andryuk
2025-08-08 14:55 ` [XEN PATCH 05/11] libxl: Convert libxl__json_parse() to use json-c Anthony PERARD
2025-08-27 15:20   ` Jason Andryuk
2025-08-08 14:55 ` [XEN PATCH 06/11] libxl: convert libxl__json_object_to_yajl_gen to libxl__json_object_to_libjsonc_object Anthony PERARD
2025-08-27 15:37   ` Jason Andryuk
2025-08-29 13:56     ` Anthony PERARD [this message]
2025-08-31 14:51       ` Jason Andryuk
2025-09-29  9:52         ` Anthony PERARD
2025-08-08 14:55 ` [XEN PATCH 07/11] libxl: libxl__object_to_json() to json-c Anthony PERARD
2025-08-27 17:51   ` Jason Andryuk
2025-08-27 17:59     ` Andrew Cooper
2025-08-29 15:32       ` Anthony PERARD
2025-08-08 14:55 ` [XEN PATCH 08/11] libxl: convert libxl__json_object_to_json() to json_object Anthony PERARD
2025-08-27 17:54   ` Jason Andryuk
2025-08-08 14:56 ` [XEN PATCH 09/11] tools/libxenstat: Use json-c when available Anthony PERARD
2025-08-27 18:00   ` Jason Andryuk
2025-08-08 14:56 ` [XEN PATCH 10/11] configure: Use json-c by default, fallback to yajl Anthony PERARD
2025-08-08 14:56 ` [XEN PATCH 11/11] Update CHANGELOG and README with dependency on json-c Anthony PERARD
2025-08-11  8:27   ` Oleksii Kurochko
2025-08-11 10:55   ` Andrew Cooper
2025-08-11 14:37     ` Anthony PERARD
2025-08-08 15:14 ` [XEN PATCH 00/11] Allow to build libxl and other tools with json-c instead of yajl Andrew Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aLGxc2d5rZspn9wj@l14 \
    --to=anthony@xenproject.org \
    --cc=anthony.perard@vates.tech \
    --cc=jason.andryuk@amd.com \
    --cc=jgross@suse.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.