Feedback wanted: Make parsing and printing configurable without adding new functions for every option

[FSMaxB]

UPDATE: Current plans are described here

In #175 (comment) I was discussing the need for a way to configure cJSON that doesn't require changing or extending the API when a new option is added and doesn't break the ABI either.

This is just an idea and I would like to get some feedback on this.

Since cJSON is a JSON library, using JSON for configuration seems sensible, not as string but rather as tree of cJSON structs.

Traversing the tree of items every time a configuration option is accessed would be a big overhead, so I think there should be a function like cJSON_CreateConfiguration or similar that takes a tree of cJSON structs and returns a dynamically allocated struct that can then be passed to cJSONs functions.

Like the following:

cJSON *configuration_json = cJSON_CreateObject();
cJSON_AddItemToObject(configuration_json, "format", cJSON_CreateBool(0));

/* hooks are of type cJSON_Hooks */
cJSON_Configuration *configuration = cJSON_CreateConfiguration(configuration_json, hooks);

/* this prints the configuration_json without formatting */
char *printed_configuration = cJSON_PrintWithConfiguration(configuration_json, configuration); 

cJSON_Configuration would just be a typedef of void*. The internal representation can be changed from version to version without breaking compatibility.

What do you think?

[ajaybhargav]

I have gone though suggestion and I thought its better to take conversation from #175 to here.

I just thought of a better idea. Why not attach options to JSON node itself? a simple function e.g.
cJSON_SetOpts(cJSON *, void *opts);
and opts can be based on node type a union of configuration settings. we can add a new member in cJSON structure as void *opt which can be assigned to respective option structure according to type(e.g. string/number etc) or to simplify we can keep a common structure say struct cJSON_Opts

Advantage of attaching opt to node is that it can be referenced anytime in the whole source and it will be completely thread safe. Moreover this can be implemented right away in existing 1.x source and carried later to 2.x

what you say?

[FSMaxB]

I'm not sure if I understand correctly. You mean adding a configuration object to every single node? In that case your cJSON_SetOpts function would have to add the options to every single child in the tree recursively, otherwise it wouldn't work with the way that cJSON prints the tree recursively.

This also won't work for cJSON_Parse and other functions.

I think the actual configuration should be passed in to every function call manually and should be intentionally left as void* and it's internals considered unstable across versions.

The use case is to create a few global configuration objects at the beginning and later passing them in as needed (or creating them on the fly, but this would make performance worse.

This is still completely thread safe and allows extensions in the future because the actual configuration is done via JSON before converting it to this void* object with cJSON_CreateConfiguration.

The configuration objects can be stored in a global variable and reused over and over after they have been created once. Passing NULL would use the default settings.

[FSMaxB]

This is something that could be done for v1 already btw.

[ajaybhargav]

Let me make it more clear... with example

/* modified structure */
typedef struct cJSON
{
    /* next/prev allow you to walk array/object chains. Alternatively, use GetArraySize/GetArrayItem/GetObjectItem */
    struct cJSON *next;
    struct cJSON *prev;
    /* An array or object item will have a child pointer pointing to a chain of the items in the array/object. */
    struct cJSON *child;

    /* The type of the item, as above. */
    int type;

    /* The item's string, if type==cJSON_String  and type == cJSON_Raw */
    char *valuestring;
    /* writing to valueint is DEPRECATED, use cJSON_SetNumberValue instead */
    int valueint;
    /* The item's number, if type==cJSON_Number */
    double valuedouble;

    /* The item's name string, if this item is the child of, or is in the list of subitems of an object. */
    char *string;
	
	/* Options for this node (optional) */
	void *opt;
} cJSON;

/* This is just dummy for explanation */

struct cJSON_NumOpts {
	int decimal_places;
	char *format; /* optional */
};

/* set opts */
void cJSON_SetOpts(cJSON *node, void *opts)
{
	/* Check node type, maybe a switch case? */
	if (node->type & cJSON_Number) {
		struct cJSON_NumOpts *numopt = opts;
		
		node->opt = global_hooks.allocate(sizeof(struct cJSON_NumOpts));
		memcpy(node->opt, opts, sizeof(struct cJSON_NumOpts));
	}
}

/* Lets create some JSON */
root = cJSON_CreateObject();
cJSON_AddItemToObject(root, "name", cJSON_CreateString("My Device"));
child = cJSON_CreateObject()
cJSON_AddItemToObject(root, "location", child);
cJSON *node = cJSON_CreateNumber(11.123456);

/* I want number to be printed with 6 decimal places */
struct cJSON_NumOpts opts = {
	.decimal_places = 6,
	.format = NULL,
}
/* set option */
cJSON_SetOpts(node, &opts);
cJSON_AddItemToObject(child, "lat", node);

node = cJSON_CreateNumber(22.123456);
/* set same option */
cJSON_SetOpts(node, &opts);
cJSON_AddItemToObject(child, "long", node);

child = cJSON_CreateNumber(22.56);
/* set only 2 decimal places for temperature */
opts.decimal_places = 2;
/* set option */
cJSON_SetOpts(child, &opts);
cJSON_AddItemToObject(root, "temperature", child);

/* add another number with no opts - regular calls... */
child = cJSON_CreateNumber(22.12312);
cJSON_AddItemToObject(root, "number", child);

Now when doing print...

/* inside function print_number */

/* check if opts are set */
if (item->opts) {
	char format[10];
	/* create a format */
	sprintf(format, "%%1.%dg", item->opts->decimal_places);
	/* Then convert it to string */
	sprintf((char*)number_buffer, format, d);
} else {
	/* do regular convertion */
}

While parsing we dont have to do anything special, opt can be kept as NULL. if someone whats to set specific option. The above is just an example but can be improved a lot and optimized later on. This will not break anything and add great functionality to the library.

More configurations can be added by extending option structures for individual cJSON_Type.

Forgot to add:

/* while deleting item */
/* check if opts are there */
if (item->opts)
	global_hooks.deallocate(item->opts);

[FSMaxB]

Adding the option to the node itself bloats the cJSON struct by 4 or 8 bytes depending on the processor architecture, maybe even more because of padding. And that even though not many people would even use it.

Furthermore this won't solve things like formatted vs. unformatted printing, unless you attach a cJSON_ArrayOpts and cJSON_ObjectOpts to every single array and object in the tree. What I want though is a general purpose way of doing configuration.

Per-Item settings as in your example would be a bit inconvenient to do when passing in the configuration on a function call level but it's still possible with cJSON_Raw by printing the nodes that are special into a new node of cJSON_Raw while specifying the configuration object. This cJSON_Raw item can then replace the original number node for printing of the entire JSON.

Regarding #175: You can even do this already, by printing the number into valuestring with your own printf and format specifier and then changing the type to cJSON_Raw.

[ajaybhargav]

The potential problem I see with generalized configuration is, what if I want configuration to be done for specific node only. Can you share what could be the possible configurations that can be generalized? So it will give a clear idea what we are thinking to do and best way to implement it.

Regarding #175: You can even do this already, by printing the number into valuestring with your own printf and format specifier and then changing the type to cJSON_Raw.

I am already doing that for now and it is fine till I am not dealing with arrays, So i felt its more convenient If there is an option to configure display format.

[FSMaxB]

The potential problem I see with generalized configuration is, what if I want configuration to be done for specific node only. Can you share what could be the possible configurations that can be generalized? So it will give a clear idea what we are thinking to do and best way to implement it.

I can't think of any way other than converting a subset of a tree to cJSON_Raw.

A possible configuration object could look like this:

{
    "buffer": 4096,
    "format": true,
    "case_sensitive": false,
    "allow_data_after_json": false
}

allow_data_after_json would do the opposite of require_null_terminated.

Regarding #175: You can even do this already, by printing the number into valuestring with your own printf and format specifier and then changing the type to cJSON_Raw.

I am already doing that for now and it is fine till I am not dealing with arrays, So i felt its more convenient If there is an option to configure display format.

It is not a problem with arrays either.

cJSON_bool CustomNumberPrinter(cJSON *number)
{
    if (!cJSON_IsNumber(number))
    {
        return 0;
    }

    /* print your number to valuestring */
    number->valuestring = /* result here */;
    
    number->type = ((number->type) & (~cJSON_Number)) | cJSON_Raw;

    return 1;
}

Do the above for every number in the array and you're done.