HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

parcel: a JSON parsing library in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
jsonparcelparsinglibrary

Problem

I've created a JSON parsing library in C, and would like some feedback on it (feel free to submit a pull request on GitHub). Any and all suggestions are acceptable, but I would prefer if reviews were focused on making this more efficient.

My header file:

#ifndef _json_H_
#define _json_H_

/**
 * JSON type identifier. Basic types are:
 *  o Object
 *  o Array
 *  o String
 *  o Other primitive: number, boolean (true/false) or null
 */
typedef enum { JSON_PRIMITIVE, JSON_OBJECT, JSON_ARRAY, JSON_STRING } JsonType;

typedef enum
{
    JSON_ERROR_NOMEM = -1, // Not enough tokens were provided
    JSON_ERROR_INVAL = -2, // Invalid character inside JSON string
    JSON_ERROR_PART = -3, // The string is not a full JSON packet, more bytes expected
    JSON_SUCCESS = 0 // Everthing is fine
} JsonError;

/**
 * JSON token description.
 * @param       type    type (object, array, string etc.)
 * @param       start   start position in JSON data string
 * @param       end     end position in JSON data string
 */
typedef struct
{
    JsonType type;
    int start;
    int end;
    int size;
    #ifdef json_PARENT_LINKS
    int parent;
    #endif
} JsonToken;

/**
 * JSON parser. Contains an array of token blocks available. Also stores
 * the string being parsed now and current position in that string
 */
typedef struct
{
    unsigned int pos; /* offset in the JSON string */
    unsigned int toknext; /* next token to allocate */
    int toksuper; /* superior token node, e.g parent object or array */
} JsonParser;

/**
 * Create JSON parser over an array of tokens
 */
void json_initJsonParser(JsonParser *parser);

/**
 * Run JSON parser. It parses a JSON data string into and array of tokens, each describing
 * a single JSON object.
 */
JsonError json_parseJson(JsonParser *parser, const char *js, JsonToken *tokens, unsigned int tokenNum);

#endif /* _json_H_ */


My C source code:

```
/**
* @file json.c
* @brief JSON Parser
*/

#include
#include "json.h"

/**
* @fn s

Solution

What you did well

  • As far as I can tell, the code produces correct output when given correct input.



  • The functions feel like a coherent library, with consistent function names and logical order of parameters.



  • You use a caller-owns-everything memory management policy, which works well for C.



What you could improve on

  • The name json_initJsonParser() is a bit redundant. json_initParser() should suffice.



  • I don't think that #ifdef json_PARENT_LINKS is a good idea. A user could easily compile the library without support for parent links, yet compile the application with json_PARENT_LINKS defined, leading to nasty bugs. Why not always enable support parent links? It would be pretty difficult to make sense of the parser output without it.



  • I think that #ifdef json_STRICT would be better as a runtime option rather than a compile-time option. If the user wants to have both modes available, there's no reasonable way to do so.



  • JSON primitives may be numbers, true, false, or null. You accept illegal primitives such as truthy.



  • json_allocJsonToken() doesn't "allocate" memory in the sense that I expect. Also, two of the three calls to json_allocJsonToken() are followed by json_fillToken(), so you might as well combine the two functions. Filling in dummy values first is senseless.



  • In the JsonParser structure, toknext and toksuper should be the same data type. It's weird that one is unsigned and the other is signed. I recommend just using int for both, or possibly ssize_t.



-
It is not obvious that the way to extract the results is to use a loop like

for (int i = 0; i < parser.toknext; i++) {
    //              ^^^^^^^^^^^^^^
}


Please feel free to add some generous comments with usage examples in the header file.

Code Snippets

for (int i = 0; i < parser.toknext; i++) {
    //              ^^^^^^^^^^^^^^
}

Context

StackExchange Code Review Q#43872, answer score: 13

Revisions (0)

No revisions yet.