[nilason]

Use G_malloc and G_free internally in Parson.

• libgrass_parson library now depends on libgrass_gis
• wrap G_malloc in private function adapting to JSON_Malloc_Function definition
• G_json_value_init_object and G_json_value_init_array always return allocated object
• parson is initialized lazily using C11 atomic operation

[wenzeslaus]

Can you please specify here what is the big picture? Like all library functions result in fatal on non-recoverable errors...

[nilason]

Can you please specify here what is the big picture? Like all library functions result in fatal on non-recoverable errors...

Yes, but not only library, modules as well use G_malloc. Current G_malloc's most advantageous feature is that it calls G_fatal_error on failure to malloc (with informative error message) and in the extension the caller of G_malloc is guaranteed to receive a non-null object, rendering return value checking superfluous (with help of __attribute__((returns_nonnull))).

It occurred to me now that stdatomic.h is only optional part of C11, although nowadays current compilers generally support it (including Visual Studio 2022). The question is whether I should guard its use, or let us see if this leads to problems "in the wild" and act accordingly at that time.

[nilason]

I added a simple fallback for C11 compiler without support for atomic operations, this adds a small risk of undefined behaviour (in case of data race). While Parson library is not thread safe in itself, the lazy initialisation adds a potential, however minuscule, risk.

[wenzeslaus]

This fits with the overall approach elsewhere, so probably fine as a consistent solution. The function scope should be limited. The text of the individual comments is mostly AI-written (reviewed by me).

[wenzeslaus]

This function is only used within gjson.c, but it has external linkage, no G_ prefix, and no declaration in a header, so it becomes an exported symbol of libgrass_parson. Making it static does not affect the allocator registration: json_set_allocation_functions() receives function pointers, and calling through a pointer is independent of symbol linkage (G__parson_malloc is already static). While at it, a short comment stating what the atomics do and do not guarantee would help future readers.

Suggested change

	void ensure_parson_initialized(void)
	/*
	* Register G_malloc/G_free with parson exactly once. The atomic logic
	* makes only this one-time registration thread-safe; parson itself has
	* unsynchronized global state and makes no thread-safety guarantees.
	*/
	static void ensure_parson_initialized(void)

[wenzeslaus]

Naming this set of values makes sense, but the name is currently unused: the variable is _Atomic int and expected below is int. Using the type makes the declaration match the intent:

Suggested change

	typedef enum { UNINITIALIZED, INITIALIZING, INITIALIZED } init_state_t;
	static _Atomic int parson_init_state = UNINITIALIZED;
	typedef enum { UNINITIALIZED, INITIALIZING, INITIALIZED } init_state_t;
	static _Atomic init_state_t parson_init_state = UNINITIALIZED;

With this, expected in ensure_parson_initialized() needs a matching type change; see the suggestion there.

[wenzeslaus]

Follow-up to using init_state_t for parson_init_state above: the compare-exchange generic function requires the expected pointer to match the atomic variable's value type, so this needs to be:

Suggested change

	int expected = UNINITIALIZED;
	init_state_t expected = UNINITIALIZED;

[wenzeslaus]

Stray blank lines inside the block:

Suggested change

	if (atomic_compare_exchange_strong(&parson_init_state, &expected,
	INITIALIZING)) {
	json_set_allocation_functions(G__parson_malloc, G_free);
	atomic_store_explicit(&parson_init_state, INITIALIZED,
	memory_order_release);
	}
	if (atomic_compare_exchange_strong(&parson_init_state, &expected,
	INITIALIZING)) {
	json_set_allocation_functions(G__parson_malloc, G_free);
	atomic_store_explicit(&parson_init_state, INITIALIZED,
	memory_order_release);
	}

[wenzeslaus]

The never-NULL guarantee is worth stating in the documentation as well, not only via the attribute:

Suggested change

	/// \since version 8.5
	/// Never returns NULL; exits with G_fatal_error() on allocation failure.
	///
	/// \since version 8.5

[wenzeslaus]

Same here:

Suggested change

	/// \since version 8.5
	/// Never returns NULL; exits with G_fatal_error() on allocation failure.
	///
	/// \since version 8.5