Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A new attempt at location information for decoded JSON elements #662

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

n0-1
Copy link

@n0-1 n0-1 commented Sep 28, 2023

This is a respin of the rejected #461 containing a reimplementation of the
feature in a less intrusive manner for library users keeping it disabled.

The nftables project is very interested in having this feature to provide
similar error reporting to its JSON API users as there is for non-JSON input.

This new implementation does not introduce any overhead on storage and
negligible on CPU cycles if not enabled: It then boils down to a few extra
function calls returning early after a simple variable value check.

The above is realized by storing json_t objects' location data in a hash table
indexed by the object's address. The table is initialized upon first insert,
which then registers a destructor function using atexit() to free up any data
at program exit (to avoid complaints from memory checkers like valgrind).

If an object is deleted, its corresponding hashtable entry is removed along
with it. To facilitate location data for simple values, the singleton 'true',
'false' and 'null' objects have to be turned into allocated data structures (so
they get a unique address in memory).

This MR contains five commits:

  1. is fallout, a previous commit missed to update tests/.gitignore.
  2. adds a function to print a format string with negative test results.
  3. introduces the new decoder flag.
  4. implements refcounted simple values.
  5. contains the actual implementation.

This macro is very similar to fail() but supports variable number of
arguments to print format strings.

Signed-off-by: Phil Sutter <[email protected]>
This will turn on recording of parsed JSON elements in input.

Signed-off-by: Phil Sutter <[email protected]>
When storing object location, the singleton true, false and null objects
are problematic since they are indistinguishable from each other.
Overcome this by introducing a refcounted variant to allocate when
JSON_STORE_LOCATION flag was given.

Signed-off-by: Phil Sutter <[email protected]>
Aid users in printing semantic errors in JSON input by providing them
with location data for each parsed JSON element. An example usage of
such data is demonstrated by nftables' error messages:

| # nft add chain mytable mychain
| Error: No such file or directory
| add chain mytable mychain
|           ^^^^^^^

To reduce overhead for library users not interested in such data, store
it only if `JSON_STORE_LOCATION' flag was passed to json_load*()
functions. It may then be retrieved using the new API function
json_get_location().

Signed-off-by: Phil Sutter <[email protected]>
@n0-1
Copy link
Author

n0-1 commented Nov 13, 2024

Rebased onto current HEAD, no functional changes intended.

This merge request is lingering uncommented for over a year now. Quite frustrating to receive no feedback at all.

Is the implemented feature really unnecessary? How do other folks report semantic errors in JSON input?

@Zer0-One
Copy link

It seems like a cool feature to me, and I appreciate that it's optional. @akheron thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants