bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#70007: [PATCH] native JSON encoder


From: Eli Zaretskii
Subject: bug#70007: [PATCH] native JSON encoder
Date: Sat, 30 Mar 2024 16:22:57 +0300

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Sat, 30 Mar 2024 12:41:31 +0100
> Cc: casouri@gmail.com,
>  70007@debbugs.gnu.org
> 
> 29 mars 2024 kl. 07.04 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > OK.  FTR, I'm not in favor of validation of unibyte strings, I just
> > suggest that we treat them as plain-ASCII: pass them through without
> > any validation, leaving the validation to the callers.
> 
> Actually we are more or less forced to validate unibyte strings as long as 
> the serialiser returns multibyte. Which we agree that it probably shouldn't, 
> but I'd first like to take some time to ensure that returning unibyte won't 
> break anything.

Yes, I was writing about the state of affairs when we change the
serializer to emit unibyte strings.

> Thank you for pushing the new JSON parser to master. I've rebased my patch 
> and cleaned it up a bit, and it now removes all uses of Jansson from json.c. 
> Since the change involves removing some Windows-specific code, perhaps you 
> would like to check that it still compiles on that platform, if you have the 
> time?

It builds and passes the tests, thanks.  But I have a couple of minor
nits below.

> +static struct symset_tbl *
> +make_symset_table (int bits, struct symset_tbl *up)
> +{
> +  int maxbits = min (SIZE_WIDTH - 2 - (word_size < 8 ? 2 : 3), 32);
> +  if (bits > maxbits)
> +    error ("out of memory"); /* Will never happen in practice.  */

This should be a call to memory_full, no?  Or if we must signal this
error here, at least make the error message more specific: mention
JSON or somesuch.

> +{
> +  double x = XFLOAT_DATA (f);
> +  if (isinf (x) || isnan (x))
> +    signal_error ("not a finite number", f);

I'd prefer a more specific error message here, like

  JSON does not allow Inf or NaN

Last, but not least: should we have a json-available-p function that
always returns non-nil, for better backward-compatibility?  Otherwise,
code out there might decide to use json.elm which is not what we want.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]