Simple JSON parser in C

Question

Here is a simple recursive descent JSON parser, not a lot of extra functionality, although it does use the expandable vector class reviewed here (Simple expandable vector in C). I did not implement any optimizations, nor a higher level access interface, it's all pretty basic. There also isn't a JSON export, just import.

The whole source is available at github (https://github.com/HarryDC/JsonParser). CMake and testing code included, I will just post the parser files here.

My main interest would be if there are more idiomatic ways to write things in C. But of course any other input is always appreciated as well.

Header

#ifndef HS_JSON_H
#define HS_JSON_H

#include "vector.h"

enum json_value_type {
    TYPE_NULL,
    TYPE_BOOL,
    TYPE_NUMBER,
    TYPE_OBJECT, // Is a vector with pairwise entries, key, value
    TYPE_ARRAY, // Is a vector, all entries are plain 
    TYPE_STRING,
    TYPE_KEY
};

typedef struct {
    int type;
    union {
        int boolean;
        double number;
        char* string;
        char* key;
        vector array;
        vector object;
    } value;
} json_value;

// Parse string into structure of json elements and values
// return 1 if successful.
int json_parse(const char* input, json_value* root);

// Free the structure and all the allocated values
void json_free_value(json_value* val);

// Convert value to string if possible, asserts if not
char* json_value_to_string(json_value* value);

// Convert value to double if possible asserts if not
double json_value_to_double(json_value* value);

// Convert value to bool if possible asserts if not
int json_value_to_bool(json_value* value);

// Convert value to vector if it's an array asserts if not
vector* json_value_to_array(json_value* value);

// Convert value to vector if it's an object, asserts if not
vector* json_value_to_object(json_value* value);

// Fetch the value with given index from root, asserts if root is not array
json_value* json_value_at(const json_value* root, size_t index);

// Fetche the value with the given key from root, asserts if root is not object
json_value * json_value_with_key(const json_value * root, const char * key);

#ifdef BUILD_TEST
void json_test_all(void);
#endif 

#endif

Implementation

#include "json.h"

#include <assert.h>
#include <ctype.h>
#include <stddef.h>
#include <stdlib.h>
#include <string.h>

static int json_parse_value(const char ** cursor, json_value * parent);

static void skip_whitespace(const char** cursor)
{
    if (**cursor == '\0') return;
    while (iscntrl(**cursor) || isspace(**cursor)) ++(*cursor);
}

static int has_char(const char** cursor, char character)
{
    skip_whitespace(cursor);
    int success = **cursor == character;
    if (success) ++(*cursor);
    return success;
}

static int json_parse_object(const char** cursor, json_value* parent)
{
    json_value result = { .type = TYPE_OBJECT };
    vector_init(&result.value.object, sizeof(json_value));

    int success = 1;
    while (success && !has_char(cursor, '}')) {
        json_value key = { .type = TYPE_NULL };
        json_value value = { .type = TYPE_NULL };
        success = json_parse_value(cursor, &key);
        success = success && has_char(cursor, ':');
        success = success && json_parse_value(cursor, &value);

        if (success) {
            vector_push_back(&result.value.object, &key);
            vector_push_back(&result.value.object, &value);
        }
        else {
            json_free_value(&key);
            break;
        }
        skip_whitespace(cursor);
        if (has_char(cursor, '}')) break;
        else if (has_char(cursor, ',')) continue;
        else success = 0;
    }

    if (success) {
        *parent = result;
    }
    else {
        json_free_value(&result);
    }

    return success;
    return 1;
}

static int json_parse_array(const char** cursor, json_value* parent)
{
    int success = 1;
    if (**cursor == ']') {
        ++(*cursor);
        return success;
    }
    while (success) {
        json_value new_value = { .type = TYPE_NULL };
        success = json_parse_value(cursor, &new_value);
        if (!success) break;
        skip_whitespace(cursor);
        vector_push_back(&parent->value.array, &new_value);
        skip_whitespace(cursor);
        if (has_char(cursor, ']')) break;
        else if (has_char(cursor, ',')) continue;
        else success = 0;
    }
    return success;
}


void json_free_value(json_value* val)
{
    if (!val) return;

    switch (val->type) {
        case TYPE_STRING:
            free(val->value.string);
            val->value.string = NULL;
            break;
        case TYPE_ARRAY:
        case TYPE_OBJECT:
            vector_foreach(&(val->value.array), (void(*)(void*))json_free_value);
            vector_free(&(val->value.array));
            break;
    }

    val->type = TYPE_NULL;
}

int json_is_literal(const char** cursor, const char* literal) {
    size_t cnt = strlen(literal);
    if (strncmp(*cursor, literal, cnt) == 0) {
        *cursor += cnt;
        return 1;
    }
    return 0;
}

static int json_parse_value(const char** cursor, json_value* parent)
{
    // Eat whitespace
    int success = 0;
    skip_whitespace(cursor);
    switch (**cursor) {
        case '\0':
            // If parse_value is called with the cursor at the end of the string
            // that's a failure
            success = 0;
            break;
        case '"':
            ++*cursor;
            const char* start = *cursor;
            char* end = strchr(*cursor, '"');
            if (end) {
                size_t len = end - start;
                char* new_string = malloc((len + 1) * sizeof(char));
                memcpy(new_string, start, len);
                new_string[len] = '\0';

                assert(len == strlen(new_string));

                parent->type = TYPE_STRING;
                parent->value.string = new_string;

                *cursor = end + 1;
                success = 1;
            }
            break;
        case '{':
            ++(*cursor);
            skip_whitespace(cursor);
            success = json_parse_object(cursor, parent);
            break;
        case '[':
            parent->type = TYPE_ARRAY;
            vector_init(&parent->value.array, sizeof(json_value));
            ++(*cursor);
            skip_whitespace(cursor);
            success = json_parse_array(cursor, parent);
            if (!success) {
                vector_free(&parent->value.array);
            }
            break;
        case 't': {
            success = json_is_literal(cursor, "true");
            if (success) {
                parent->type = TYPE_BOOL;
                parent->value.boolean = 1;
            }
            break;
        }
        case 'f': {
            success = json_is_literal(cursor, "false");
            if (success) {
                parent->type = TYPE_BOOL;
                parent->value.boolean = 0;
            }
            break;
        }
        case 'n':
            success = json_is_literal(cursor, "null");
            break;
        default: {
            char* end;
            double number = strtod(*cursor, &end);
            if (*cursor != end) {
                parent->type = TYPE_NUMBER;
                parent->value.number = number;
                *cursor = end;
                success = 1;
            }
        }
    }

    return success;
}


int json_parse(const char* input, json_value* result)
{
    return json_parse_value(&input, result);
}

char* json_value_to_string(json_value* value)
{
    assert(value->type == TYPE_STRING);
    return (char *)value->value.string;
}

double json_value_to_double(json_value* value)
{
    assert(value->type == TYPE_NUMBER);
    return value->value.number;
}

int json_value_to_bool(json_value* value)
{
    assert(value->type == TYPE_BOOL);
    return value->value.boolean;
}

vector* json_value_to_array(json_value* value)
{
    assert(value->type == TYPE_ARRAY);
    return &value->value.array;
}

vector* json_value_to_object(json_value* value)
{
    assert(value->type == TYPE_OBJECT);
    return &value->value.object;
}

json_value* json_value_at(const json_value* root, size_t index)
{
    assert(root->type == TYPE_ARRAY);
    if (root->value.array.size < index) {
        return vector_get_checked(&root->value.array,index);
    }
    else {
        return NULL;
    }
}

json_value* json_value_with_key(const json_value* root, const char* key)
{
    assert(root->type == TYPE_OBJECT);
    json_value* data = (json_value*)root->value.object.data;
    size_t size = root->value.object.size;
    for (size_t i = 0; i < size; i += 2)
    {
        if (strcmp(data[i].value.string, key) == 0)
        {
            return &data[i + 1];
        }
    }
    return NULL;
}

#ifdef BUILD_TEST

#include <stdio.h>

void json_test_value_string(void)
{
    printf("json_parse_value_string: ");
    // Normal parse, skip whitespace
    const char* string = "     \n\t\"Hello World!\"";
    json_value result = { .type = TYPE_NULL };
    assert(json_parse_value(&string, &result));
    assert(result.type == TYPE_STRING);
    assert(result.value.string != NULL);
    assert(strlen(result.value.string) == 12);
    assert(strcmp("Hello World!", result.value.string) == 0);

    json_free_value(&result);

    // Empty string
    string = "\"\"";
    json_parse_value(&string, &result);
    assert(result.type == TYPE_STRING);
    assert(result.value.string != NULL);
    assert(strlen(result.value.string) == 0);
    json_free_value(&result);

    printf(" OK\n");
}

void json_test_value_number(void)
{
    printf("json_test_value_number: ");
    const char* string = "  23.4";
    json_value result = { .type = TYPE_NULL };
    assert(json_parse_value(&string, &result));
    assert(result.type == TYPE_NUMBER);
    assert(result.value.number == 23.4);

    json_free_value(&result);
    printf(" OK\n");
}

void json_test_value_invalid(void)
{
    printf("json_test_value_invalid: ");
    {
        // not a valid value
        const char* string = "xxx";
        json_value result = { .type = TYPE_NULL };
        assert(!json_parse_value(&string, &result));
        assert(result.type == TYPE_NULL);
        json_free_value(&result);
    }
    {
        // parse_value at end should fail
        const char* string = "";
        json_value result = { .type = TYPE_NULL };
        assert(!json_parse_value(&string, &result));
        assert(result.type == TYPE_NULL);
        json_free_value(&result);
    }
    printf(" OK\n");
}

void json_test_value_array(void)
{
    printf("json_test_value_array: ");
    {
        // Empty Array
        const char* string = "[]";
        json_value result = { .type = TYPE_NULL };
        assert(result.value.array.data == NULL);
        assert(json_parse_value(&string, &result));
        assert(result.type = TYPE_ARRAY);
        assert(result.value.array.data != NULL);
        assert(result.value.array.size == 0);
        json_free_value(&result);
    }

    {
        // One Element
        const char* string = "[\"Hello World\"]";
        json_value result = { .type = TYPE_NULL };
        assert(result.value.array.data == NULL);
        assert(json_parse_value(&string, &result));
        assert(result.type = TYPE_ARRAY);
        assert(result.value.array.data != NULL);
        assert(result.value.array.size == 1);

        json_value* string_value = (json_value *)result.value.array.data;
        assert(string_value->type == TYPE_STRING);
        assert(strcmp("Hello World", string_value->value.string) == 0);;

        json_free_value(&result);
    }

    {
        // Mutliple Elements
        const char* string = "[0, 1, 2, 3]";
        json_value result = { .type = TYPE_NULL };
        assert(result.value.array.data == NULL);
        assert(json_parse_value(&string, &result));
        assert(result.type = TYPE_ARRAY);
        assert(result.value.array.data != NULL);
        assert(result.value.array.size == 4);

        json_free_value(&result);
    }

    {
        // Failure
        const char* string = "[0, 2,,]";
        json_value result = { .type = TYPE_NULL };
        assert(result.value.array.data == NULL);
        assert(result.type == TYPE_NULL);
        assert(result.value.array.data == NULL);

        json_free_value(&result);
    }
    {
        // Failure
        // Shouldn't need to free, valgrind shouldn't show leak
        const char* string = "[0, 2, 0";
        json_value result = { .type = TYPE_NULL };
        assert(result.value.array.data == NULL);
        assert(result.type == TYPE_NULL);
        assert(result.value.array.data == NULL);
    }


    printf(" OK\n");
}

void json_test_value_object(void)
{
    printf("json_test_value_object: ");
    {
        // Empty Object
        const char* string = "{}";
        json_value result = { .type = TYPE_NULL };
        assert(result.value.object.data == NULL);
        assert(json_parse_value(&string, &result));
        assert(result.type = TYPE_OBJECT);
        assert(result.value.array.data != NULL);
        assert(result.value.array.size == 0);
        json_free_value(&result);
    }

    {
        // One Pair
        const char* string = "{ \"a\"  :   1  }";
        json_value result = { .type = TYPE_NULL };
        assert(result.value.object.data == NULL);
        assert(json_parse_value(&string, &result));
        assert(result.type = TYPE_OBJECT);
        assert(result.value.array.data != NULL);
        assert(result.value.array.size == 2);

        json_value* members = (json_value *)result.value.object.data;

        assert(strcmp(json_value_to_string(members), "a") == 0);
        ++members;
        assert(json_value_to_double(members) == 1.0);
        json_free_value(&result);
    }

    {
        // Multiple Pairs
        const char* string = "{ \"a\": 1, \"b\" : 2, \"c\" : 3 }";
        json_value result = { .type = TYPE_NULL };
        assert(result.value.object.data == NULL);
        assert(json_parse_value(&string, &result));
        assert(result.type = TYPE_OBJECT);
        assert(result.value.array.data != NULL);
        assert(result.value.array.size == 6);

        json_value* members = (json_value *)result.value.object.data;

        assert(strcmp(json_value_to_string(&members[4]), "c") == 0);
        assert(json_value_to_double(&members[5]) == 3.0);
        json_free_value(&result);
    }

    printf(" OK\n");
}

void json_test_value_literal(void) {
    printf("json_test_values_literal: ");
    {
        const char* string = "true";
        json_value result = { .type = TYPE_NULL };
        assert(json_parse_value(&string, &result));
        assert(result.type == TYPE_BOOL);
        assert(result.value.boolean);
        json_free_value(&result);
    }

    {
        const char* string = "false";
        json_value result = { .type = TYPE_NULL };
        assert(json_parse_value(&string, &result));
        assert(result.type == TYPE_BOOL);
        assert(!result.value.boolean);
        json_free_value(&result);
    }

    {
        const char* string = "null";
        json_value result = { .type = TYPE_NULL };
        assert(json_parse_value(&string, &result));
        assert(result.type == TYPE_NULL);
        json_free_value(&result);
    }
    printf(" OK\n");
}

const char* test_string_valid = " \
{ \"item1\" : [1, 2, 3, 4], \
  \"item2\" : { \"a\" : 1, \"b\" : 2, \"c\" : 3 }, \
  \"item3\" : \"An Item\" \
}";


const char* test_string_invalid = " \
{ \"item1\" : [1, 2, 3, 4], \
  \"item2\" : { \"a\" : 1, \"b\" : 2, \"c\" : 3 }, \
  \"item3\" , \"An Item\" \
}";



void json_test_coarse(void)
{
    printf("json_test_coarse: ");

    json_value root;
    assert(json_parse(test_string_valid, &root));

    json_value* val = json_value_with_key(&root, "item1");

    assert(root.type == TYPE_OBJECT);
    assert(root.value.object.size == 6);

    assert(val != NULL);
    assert(json_value_to_array(val) != NULL);
    assert(json_value_to_array(val)->size == 4);

    val = json_value_with_key(&root, "item3");
    assert(val != NULL);
    assert(json_value_to_string(val) != NULL);
    assert(strcmp(json_value_to_string(val), "An Item") == 0);

    json_free_value(&root);

    // valgrind check for releasing intermediary data
    assert(!json_parse(test_string_invalid, &root));

    printf(" OK\n");
}


void json_test_all(void)
{
    json_test_value_invalid();
    json_test_value_string();
    json_test_value_number();
    json_test_value_array();
    json_test_value_object();
    json_test_value_literal();
    json_test_coarse();
}

#endif 

Answer 1

Header

In C, all enum names share the same namespace with each other (and with things like variable names). It's therefore a good idea to try to reduce the risk that they'll collide.

Your enum json_value_type names have the prefix TYPE_, which is pretty generic. Some other library might try to use the same name. I'd suggest changing that prefix to, say, JSON_.

Also, you don't seem to be using TYPE_KEY for anything. Just remove it.

Implementation

As Roland Illig notes, the arguments to iscntrl() and isspace() in your skip_whitespace() function should be cast to unsigned char to avoid sign extension.

Alternatively, and more closely following the JSON spec, you could rewrite this function simply as:

static void skip_whitespace(const char** cursor)
{
    while (**cursor == '\t' || **cursor == '\r' ||
           **cursor == '\n' || **cursor == ' ') ++(*cursor);
}

---

A lot of your static helper functions do non-trivial combinations of things, and lack any comment explaining what they do. One or two comment lines before each function could help readability a lot.

In particular, your has_char() function does a bunch of different things:

1. It skips whitespace.
2. It checks for the presence of a certain character in the input.
3. If the character is found, it automatically skips it.

Only #2 is obviously implied by the function name; the others are unexpected side effects, and should at least be clearly documented.

Actually, it seems to me that it might be better to remove the call to skip_whitespace() from has_char(), and just let the caller explicitly skip whitespace before calling it if needed. In many cases your code already does that, making the duplicate skip redundant.

Also, to make effect #3 less surprising to the reader, it might be a good idea to rename that function to something a bit more active like, say, read_char().

---

At the end of json_parse_object(), you have:

    return success;
    return 1;
}

Surely that's redundant. Just get rid of the return 1;.

Also, it looks like you're using the generic json_parse_value() function to parse object keys, and don't test to make sure that they're strings. This allows some invalid JSON to get through your parser. I'd suggest either adding a explicit type check or splitting your string parsing code into a separate function (as described below) and calling it directly from json_parse_object().

---

At the top of json_parse_array(), you have:

if (**cursor == ']') {
    ++(*cursor);
    return success;
}
while (success) {

You could rewrite that the same way as you do in json_parse_object():

while (success && !has_char(']')) {

(Only, you know, I still think the name read_char() would be better.)

Also, for some reason, your json_parse_array() seems to expect the caller to initialize the parent struct, while json_parse_object() does it automatically. AFAICT there's no reason for the inconsistency, so you could and probably should just make both functions work the same way.

---

Your json_is_literal() function is not marked as static, even though it doesn't appear in the header. Like is_char(), I'd also prefer to rename it to something more active, like json_read_literal() or just read_literal(), to make it clearer that it automatically advances the cursor on a successful match.

(Also note that, as written, this function does not check that the literal in the input actually ends where it's supposed to. For example, it would successfully match the input nullnullnull against null. I don't think that's an actual bug, since the only valid literals in JSON are true, false and null, none of which are prefixes of each other, and since two literals cannot appear consecutively in valid JSON without some other token in between. But it's definitely at least worth noting in a comment.)

---

You might also want to explicitly mark some of your static helper functions as inline to give the compiler a hint that it should try to merge them into the calling code. I'd suggest doing that at least for skip_whitespace(), has_char() and json_is_literal().

Since your json_value_to_X() accessor functions all consist of nothing but an assert() and a pointer dereference, you should also consider moving their implementations into json.h and marking them as static inline. This would allow the compiler to inline them into the calling code even in other .c files, and possibly to optimize away the assert() if the calling code already checks the type anyway.

---

In your main json_parse() function, you might want to explicitly check that there's nothing but whitespace left in the input after the root value has been parsed.

String parsing

Your string parsing code in json_parse_value() is broken, since it doesn't handle backslash escapes. For example, it fails on the following valid JSON input:

"I say: \"Hello, World!\""

You may want to add that as a test case.

You should also test that your code correctly handles other backslash escape sequences like \b, \f, \n, \r, \t, \/ and especially \\ and \unnnn. Here's few more test cases for those:

"\"\b\f\n\r\t\/\\"
"void main(void) {\r\n\tprintf(\"I say: \\\"Hello, World!\\\"\\n\");\r\n}"
"\u0048\u0065\u006C\u006C\u006F\u002C\u0020\u0057\u006F\u0072\u006C\u0064\u0021"
"\u3053\u3093\u306B\u3061\u306F\u4E16\u754C"

Since JSON strings can contain arbitrary Unicode characters, you'll need to decide how to handle them. Probably the simplest choice would be to declare your input and output to be in UTF-8 (or perhaps WTF-8) and to convert \unnnn escapes into UTF-8 byte sequences (and, optionally, vice versa). Note that, since you're using null-terminated strings, you may prefer to decode \u0000 into the overlong encoding "\xC0\x80" instead of a null byte.

---

For the sake of keeping the main json_parse_value() function readable, I would strongly recommend splitting the string parsing code into a separate helper function. Especially since making it handle backslash escapes correctly will complicate it considerably.

One of the complications is that you won't actually know how long the string will be until you've parsed it. One way to deal with that would be to dynamically grow the allocated output string with realloc(), e.g. like this:

// resize output buffer *buffer to new_size bytes
// return 1 on success, 0 on failure
static int resize_buffer(char** buffer, size_t new_size) {
    char *new_buffer = realloc(*buffer, new_size);
    if (new_buffer) {
        *buffer = new_buffer;
        return 1;
    }
    else return 0;
}

// parse a JSON string value
// expects the cursor to point after the initial double quote
// return 1 on success, 0 on failure
static int json_parse_string(const char** cursor, json_value* parent) {
    int success = 1;

    size_t length = 0, allocated = 8;  // start with an 8-byte buffer 
    char *new_string = malloc(allocated);
    if (!new_string) return 0;

    while (success && **cursor != '"') {
        if (**cursor == '\0') {
            success = 0;  // unterminated string
        }
        // we're going to need at least one more byte of space
        while (success && length + 1 > allocated) {
             success = resize_buffer(&new_string, allocated *= 2);
        }
        if (!success) break;
        if (**cursor != '\\') {
             new_string[length++] = **cursor;  // just copy normal bytes to output
             ++(*cursor);
        }
        else switch ((*cursor)[1]) {
            case '\\':new_string[length++] = '\\'; *cursor += 2; break;
            case '/': new_string[length++] = '/';  *cursor += 2; break;
            case '"': new_string[length++] = '"';  *cursor += 2; break;
            case 'b': new_string[length++] = '\b'; *cursor += 2; break;
            case 'f': new_string[length++] = '\f'; *cursor += 2; break;
            case 'n': new_string[length++] = '\n'; *cursor += 2; break;
            case 'r': new_string[length++] = '\r'; *cursor += 2; break;
            case 't': new_string[length++] = '\t'; *cursor += 2; break;
            case 'u':
                // TODO: handle Unicode escapes! (decode to UTF-8?)
                // note that this may require extending the buffer further
            default:
                success = 0; break;  // invalid escape sequence
        }
    }
    success = success && resize_buffer(&new_string, length+1);
    if (!success) { 
        free(new_string);
        return 0;
    }
    new_string[length] = '\0';
    parent->type = TYPE_STRING;
    parent->value.string = new_string;
    ++(*cursor);  // move cursor after final double quote
    return 1;
}

An alternative solution would be to run two parsing passes over the input: one just to determine the length of the output string, and another to actually decode it. This would be most easily done something like this:

static int json_parse_string(const char** cursor, json_value* parent) {
    char *tmp_cursor = *cursor;

    size_t length = (size_t)-1;
    if (!json_string_helper(&tmp_cursor, &length, NULL)) return 0;

    char *new_string = malloc(length);
    if (!new_string) return 0;

    if (!json_string_helper(&tmp_cursor, &length, new_string)) {
        free(new_string);
        return 0;
    }
    parent->type = TYPE_STRING;
    parent->value.string = new_string;
    *cursor = tmp_cursor;
    return 1;
}

where the helper function:

static int json_parse_helper(const char** cursor, size_t* length, char* new_string) {
    // ...
}

parses a JSON string of at most *length bytes into new_string and writes the actual length of the parsed string into *length, or, if new_string == NULL, just determines the length of the string without actually storing the decoded output anywhere.

Number parsing

Your current json_parse_value() implementation treats numbers as the default case, and simply feeds anything that doesn't being with ", [, {, n, t or f into the C standard library function strtod().

Since strtod() accepts a superset of valid JSON number literals, this should work, but can make your code sometimes accept invalid JSON as valid. For example, your code will accept +nan, -nan, +inf and -inf as valid numbers, and will also accept hexadecimal notation like 0xABC123. Also, as the strtod() documentation linked above notes:

In a locale other than the standard "C" or "POSIX" locales, this function may recognize additional locale-dependent syntax.

If you want to be stricter, you might want to explicitly validate anything that looks like a number against the JSON grammar before passing it to strtod().

Also note that strtod() may set errno e.g. if the input number is outside the range of a double. You probably should be checking for this.

Testing

I have not looked at your tests in detail, but it's great to see that you have them (even if, as noted above, their coverage could be improved).

Personally, though, I'd prefer to move the tests out of the implementation into a separate source file. This does have both advantages and disadvantages:

• The main disadvantage is that you can no longer directly test static helper functions. However, given that your public API looks clean and comprehensive, and doesn't suffer from any "hidden state" issues that would complicate testing, you should be able to achieve good test coverage even just through the API.
• The main advantage (besides a clean separation between implementation and testing code) is that your tests will automatically test the public API. In particular, any problems with the json.h header will show up in your tests. Also, doing your tests via the API helps you ensure that your API really is sufficiently complete and flexible for general use.

If you really still want to directly test your static functions, you could always add a preprocessor flag that optionally exposes them for testing, either via simple wrappers or just by removing the static keyword from their definitions.

Ps. I did notice that your json_test_value_number() test is failing for me (GCC 5.4.0, i386 arch), presumably because the number 23.4 is not exactly representable in floating point. Changing it to 23.5 makes the test pass.

Answer 2

This is in no way a complete review, but I'll share some things that caught my eye while reading your code.

Comments

While comments surely are nice, some of your inline comments add only noise to the code.

// Eat whitespace
int success = 0;
skip_whitespace(cursor);

First of all, the comment is one line too early. Second, one can read that the whitespace is consumed by looking at the function - the name describes it perfectly, there's no need for an additional comment.

case '\0':
    // If parse_value is called with the cursor at the end of the string
    // that's a failure
    success = 0;
    break;

Again, this comment just repeats what the code itself is saying.

---

enum json_value_type {
    TYPE_NULL,
    TYPE_BOOL,
    TYPE_NUMBER,
    TYPE_OBJECT, // Is a vector with pairwise entries, key, value
    TYPE_ARRAY, // Is a vector, all entries are plain 
    TYPE_STRING,
    TYPE_KEY
};

Now, these comments are not really useless since they document what each value represents. But why only for TYPE_OBJECT and TYPE_ARRAY - why not for all values? Personally, I'd just put a link to json.org just before that enum. Your types are analogous to the ones there, you need only document what TYPE_KEY is supposed to be. Which brings me to the next point...

TYPE_KEY

Taking a look at json.org, you can see an object consists of a list of members, which in turn are made of a string and a value. Which means that you don't really need TYPE_KEY! Just add a new struct for members consisting of a TYPE_STRING value and another json value of any type and you're good to go. Right now, you could have e.g. a number as key for a value, which is not allowed. Would make some of the object-related logic nicer too, like this for loop:

for (size_t i = 0; i < size; i += 2)

Ironically, the step of this for loop actually could use a comment (why += 2?) but lacks one.

Miscellaneous

case '\0':
    // If parse_value is called with the cursor at the end of the string
    // that's a failure
    success = 0;
    break;

Why not just return 0;?

---

while (iscntrl(**cursor) || isspace(**cursor)) ++(*cursor);

and

if (success) ++(*cursor);

and

if (has_char(cursor, '}')) break;
else if (has_char(cursor, ',')) continue;

and a few others of those. I'm not particularly fond of putting condition and statement on the same line, especially since you're not consistently doing this. I'm kinda okay with doing this for the sake of control flow, like if (!something) return;, but it's still "meh". Better do it right and put the statement on a new line.

---

Also, I find that your code could use some more empty lines to seperate "regions" or whatever you'd like to call them. For example:

json_value key = { .type = TYPE_NULL };
json_value value = { .type = TYPE_NULL };
success = json_parse_value(cursor, &key);
success = success && has_char(cursor, ':');
success = success && json_parse_value(cursor, &value);

if (success) {
    vector_push_back(&result.value.object, &key);
    vector_push_back(&result.value.object, &value);
}
else {
    json_free_value(&key);
    break;
}
skip_whitespace(cursor);
if (has_char(cursor, '}')) break;
else if (has_char(cursor, ',')) continue;
else success = 0;

There is one empty line seperating the setup-and-parse-stuff from the check-and-return stuff, but you can do better.

json_value key = { .type = TYPE_NULL };
json_value value = { .type = TYPE_NULL };

success = json_parse_value(cursor, &key);
success = success && has_char(cursor, ':');
success = success && json_parse_value(cursor, &value);

if (success) {
    vector_push_back(&result.value.object, &key);
    vector_push_back(&result.value.object, &value);
}
else {
    json_free_value(&key);
    break;
}

skip_whitespace(cursor);

if (has_char(cursor, '}')) break;
else if (has_char(cursor, ',')) continue;
else success = 0;

I find this to be way cleaner. You have a block for setting up the values, a block for parsing them, a block for putting them into the vector, a block for skipping whitespace and a block for finalizing the current action. The last empty line between skip_whitespace(cursor); and if ... is debatable, but I prefer it this way.

---

Other than that, I found your code to be easily readable and understandable. You properly check for any errors and use sensible naming. As for the idiomaticity, apart from what I've mentioned, there's nothing I'd mark as unusual or un-idomatic.

Answer 3

The functions from ctype.h must not be called with arguments of type char, since that can invoke undefined behavior. See the NetBSD documentation for a good explanation.