Prosty parser JSON w C

Oto prosty rekurencyjny analizator składni JSON, niezbyt wiele dodatkowych funkcji, chociaż korzysta z rozszerzalnej klasy wektorowej omówionej tutaj ( Prosty rozszerzalny wektor w C ). Nie zaimplementowałem żadnych optymalizacji ani interfejsu dostępu wyższego poziomu, wszystko jest dość proste. Nie ma też eksportu JSON, tylko import.

Całe źródło jest dostępne na github ( https://github.com/HarryDC/JsonParser ). Zawiera CMake i kod testowy, po prostu umieszczę tutaj pliki parsera.

Moim głównym zainteresowaniem byłoby, gdyby istniały bardziej idiomatyczne sposoby pisania rzeczy w C. Ale oczywiście każdy inny wkład jest zawsze mile widziany.

Nagłówek

#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 

Implementacja

#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 

Odpowiedź

Nagłówek

W języku C wszystkie enum nazwy mają tę samą przestrzeń nazw ( i rzeczy takie jak nazwy zmiennych). Dlatego dobrym pomysłem jest zmniejszenie ryzyka zderzenia.

Twoje enum json_value_type nazwy mają prefiks TYPE_, co jest dość ogólne. Inna biblioteka może próbować użyć tej samej nazwy. Proponuję zmienić ten prefiks na, powiedzmy, JSON_.

Poza tym wydaje się, że nie używasz TYPE_KEY na wszystko. Po prostu go usuń.

Implementacja

Jak zauważa Roland Illig , argumenty iscntrl() i isspace() w funkcji skip_whitespace() powinny być rzutowane na unsigned char do unikaj rozszerzenia znaku.

Alternatywnie i dokładniej zgodnie ze specyfikacją JSON , możesz przepisać tę funkcję po prostu jako:

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

Wiele funkcji pomocniczych static wykonuje nietrywialne kombinacje rzeczy i brakuje im komentarza wyjaśniającego, co zrobić. Jeden lub dwa wiersze komentarza przed każdą funkcją mogą bardzo zwiększyć czytelność.

W szczególności funkcja has_char() wykonuje wiele różnych rzeczy:

  1. Pomija białe znaki.
  2. Sprawdza obecność określonego znaku na wejściu.
  3. Jeśli znak zostanie znaleziony, automatycznie go pomija.

Z nazwy funkcji wynika oczywiście tylko # 2; inne są nieoczekiwanymi skutkami ubocznymi i powinny być przynajmniej jasno udokumentowane.

Właściwie wydaje mi się, że lepiej byłoby usunąć wywołanie skip_whitespace() z has_char() i po prostu pozwól wywołującemu jawnie pominąć białe znaki przed wywołaniem go, jeśli to konieczne. W wielu przypadkach twój kod już to robi, dzięki czemu duplikat pomijania jest zbędny.

Ponadto, aby efekt nr 3 był mniej zaskakujący dla czytelnika, dobrym pomysłem może być zmiana nazwy tej funkcji na coś bardziej aktywne, np. read_char().


Na końcu json_parse_object() masz:

 return success; return 1; } 

Z pewnością jest to zbędne. Po prostu pozbądź się return 1;.

Ponadto, wygląda na to, że „używasz ogólnej funkcji json_parse_value() do analizowania kluczy obiektów i nie testuj, aby upewnić się, że są to ciągi. To pozwala niektórym nieprawidłowym JSON przejść przez twój parser. Proponuję albo dodanie jawnego sprawdzenia typu, albo podzielenie kodu parsującego ciąg na osobną funkcję (jak opisano poniżej) i wywołanie go bezpośrednio z json_parse_object().


Na górze json_parse_array() masz:

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

Możesz przepisać to tak samo jak robisz w json_parse_object():

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

(Tylko, wiesz, nadal myślę, że nazwa byłoby lepsze.)

Ponadto, z jakiegoś powodu, Twój json_parse_array() wydaje się oczekiwać, że dzwoniący zainicjuje parent struct, podczas gdy json_parse_object() robi to automatycznie. AFAICT nie ma powodu do niespójności, więc możesz i prawdopodobnie powinieneś po prostu sprawić, by obie funkcje działały w ten sam sposób.


Twoja funkcja json_is_literal() nie jest oznaczona jako static, mimo że nie pojawiają się w nagłówku. Podobnie jak is_char(), nie mam dlatego wolę zmienić nazwę na bardziej aktywną, na przykład json_read_literal() lub po prostu read_literal(), aby było wyraźniej, że automatycznie przesuwa kursor do pomyślne dopasowanie.

(Zauważ też, że, jak napisano, ta funkcja nie sprawdza, czy literał w wejściu faktycznie kończy się tam, gdzie powinien. Na przykład z powodzeniem dopasuje dane wejściowe nullnullnull do null.Nie sądzę, żeby to był rzeczywisty błąd, ponieważ jedyne prawidłowe literały w JSON to true, false i null, z których żaden nie jest przedrostkiem siebie nawzajem, a ponieważ dwa literały nie mogą pojawiać się kolejno w prawidłowym formacie JSON bez innego tokenu pomiędzy nimi. Ale zdecydowanie warto to przynajmniej odnotować w komentarzu.)


Możesz również wyraźnie oznaczyć niektóre ze swoich statycznych funkcji pomocniczych jako inline aby dać kompilatorowi wskazówkę, że powinien spróbować połączyć je z kodem wywołującym. Proponuję zrobić to przynajmniej dla skip_whitespace(), has_char() i json_is_literal().

Ponieważ funkcje akcesora json_value_to_X() składają się wyłącznie z assert() i wyłuskiwanie wskaźnika, należy również rozważyć przeniesienie ich implementacji do json.h i oznaczenie ich jako static inline. Pozwoliłoby to kompilatorowi wstawić je do kodu wywołującego nawet w innych plikach .c i prawdopodobnie zoptymalizować assert(), jeśli wywołanie kod i tak już sprawdza typ.


W swojej głównej funkcji json_parse() możesz wyraźnie sprawdzić, czy nie ma nic poza spacjami w dane wejściowe po przeanalizowaniu wartości głównej.

Analiza ciągu

Kod analizy ciągu w json_parse_value() jest uszkodzony, ponieważ nie „t obsługuje znaki z ukośnikiem odwrotnym. Na przykład kończy się niepowodzeniem przy następujących prawidłowych danych wejściowych JSON:

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

Możesz dodać to jako przypadek testowy.

Możesz należy również sprawdzić, czy kod poprawnie obsługuje inne sekwencje specjalne z ukośnikiem odwrotnym, takie jak \b, \f, \n, \r, \t, \/, a zwłaszcza \\ i \unnnn. Oto kilka innych przypadków testowych dla nich:

"\"\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" 

Ponieważ łańcuchy JSON mogą zawierać dowolne znaki Unicode, „będziesz musiał zdecydować, jak je obsłużyć. Prawdopodobnie najprostszym wyborem byłoby zadeklarowanie danych wejściowych i wyjściowych w formacie UTF-8 (lub może WTF-8 ) i przekonwertowanie \unnnn przechodzi do sekwencji bajtów UTF-8 (i opcjonalnie odwrotnie). Zwróć uwagę, że ponieważ „używasz ciągów zakończonych znakiem null, możesz preferować zdekodowanie \u0000 do zbyt długiego kodowania "\xC0\x80" zamiast bajtu zerowego.


W celu zachowania czytelności głównej funkcji json_parse_value(), Zdecydowanie zalecałbym podzielenie kodu analizującego ciągi na osobną funkcję pomocniczą. Zwłaszcza, że poprawne obsługiwanie znaków z ukośnikiem odwrotnym znacznie go skomplikuje.

Jedną z komplikacji jest to, że nie wiesz, jak długo ciąg będzie istniał, dopóki go nie przeanalizujesz. Jednym ze sposobów radzenia sobie z tym byłoby dynamiczne powiększanie przydzielonego ciągu wyjściowego za pomocą realloc(), np. w ten sposób:

// 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; } 

Alternatywnym rozwiązaniem byłoby uruchomienie dwóch przebiegów analizowania danych wejściowych: jednego tylko po to, aby określić długość ciągu wyjściowego, a drugiego, aby go faktycznie zdekodować. Najłatwiej byłoby to zrobić coś takiego:

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; } 

gdzie funkcja pomocnicza:

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

analizuje ciąg JSON o maksymalnej wartości *length bajtów na new_string i zapisuje rzeczywistą długość przeanalizowanego ciągu w *length lub if new_string == NULL, po prostu określa długość ciągu bez faktycznego przechowywania zdekodowanego wyjścia w dowolnym miejscu.

Analiza liczb

Twój obecny json_parse_value() traktuje liczby jako przypadek domyślny i po prostu podaje wszystko, co nie jest z ", [, {, n, t lub f do standardowej funkcji biblioteki C strtod().

Ponieważ strtod() akceptuje nadzbiór prawidłowych literałów liczbowych JSON, to powinno działać, ale czasami może sprawić, że kod cept nieprawidłowy JSON jako prawidłowy. Na przykład Twój kod zaakceptuje +nan, -nan, +inf i jako prawidłowe liczby, a także akceptuje zapis szesnastkowy, taki jak 0xABC123. Ponadto, zgodnie z powyższymi uwagami w dokumentacji strtod(), do których prowadzi łącze:

W języku innym niż standardowe „C” lub ustawienia regionalne „POSIX”, ta funkcja może rozpoznawać dodatkową składnię zależną od ustawień regionalnych.

Jeśli chcesz być bardziej rygorystyczny, możesz jawnie zweryfikować wszystko, co wygląda jak liczba, z gramatyka JSON przed przekazaniem jej do strtod().

Pamiętaj też, że strtod() może ustawić errno np jeśli wprowadzona liczba jest poza zakresem double. Prawdopodobnie powinieneś to sprawdzić.

Testowanie

Nie przeglądałem szczegółowo twoich testów, ale wspaniale jest widzieć, że je masz (nawet jeśli, jak zaznaczono powyżej, ich zasięg można by poprawić).

Osobiście wolałbym jednak przenieść testy z implementacji do oddzielnego pliku źródłowego. Ma to zarówno zalety, jak i wady:

  • Główną wadą jest to, że nie można już bezpośrednio testować statycznych funkcji pomocniczych. Jednak biorąc pod uwagę, że Twój publiczny interfejs API wygląda na czysty i wszechstronny oraz nie ma żadnych problemów związanych z „stanem ukrytym”, które utrudniałyby testowanie, powinieneś być w stanie osiągnąć dobre pokrycie testów nawet przez samo API.
  • Główną zaletą (oprócz czystego oddzielenia kodu implementacyjnego od testowego) jest to, że Twoje testy będą automatycznie testować publiczny interfejs API. W szczególności wszelkie problemy z nagłówkiem json.h pojawią się w testy. Ponadto wykonanie testów za pośrednictwem interfejsu API pomaga upewnić się, że interfejs API jest naprawdę wystarczająco kompletny i elastyczny do ogólnego użytku.

Jeśli naprawdę nadal chcesz bezpośrednio testować swoje funkcje statyczne, zawsze możesz dodać flagę preprocesora, która opcjonalnie wystawia je do testowania, albo poprzez proste opakowania, albo po prostu usuwając słowo kluczowe static z ich definicji.

Ps. I zauważyłeś, że Twój test json_test_value_number() kończy się niepowodzeniem (GCC 5.4.0, arch i386), prawdopodobnie ponieważ liczba 23,4 nie jest dokładnie reprezentowalna w postaci zmiennoprzecinkowej. Zmiana na 23.5 sprawia, że test przechodzi pomyślnie.

Komentarze

  • Świetna robota, dzięki, oczywiście nie zrobiłem ' t sprawdź standard tak bardzo, jak powinienem. Chociaż celowo zignorowałem utf-8 (prawdopodobnie powinienem był o tym wspomnieć.
  • OK, w porządku. Powinieneś nadal przynajmniej dekodować proste znaki ucieczki w postaci \u00XX, ponieważ niektóre kodery mogą zdecydować się na ich użycie nawet dla znaków ASCII. (Dodałem również sugestię, aby oznaczyć niektóre funkcje jako inline powyżej, ponieważ zapomniałem to zrobić wcześniej .)
  • Całkowicie przegapiłem tę część standardu, tak, sekwencje specjalne muszą być poprawnie przeanalizowane.

Odpowiedź

W żadnym wypadku nie jest to pełna recenzja, ale podzielę się niektórymi rzeczami, które zwróciły moją uwagę podczas czytania Twojego kodu.

Komentarze

Chociaż komentarze z pewnością są miłe, niektóre z twoich wbudowanych komentarzy powodują tylko szum w kodzie.

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

Po pierwsze, komentarz jest o jedną linię za wcześnie. Po drugie, można przeczytać, że spacja jest konsumowana patrząc na funkcję – nazwa doskonale ją opisuje, th Nie ma potrzeby dodatkowego komentarza.

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

Ponownie, ten komentarz po prostu powtarza to, co mówi sam kod.


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 }; 

Te komentarze nie są tak naprawdę bezużyteczne, ponieważ dokumentują, co reprezentuje każda wartość. Ale dlaczego tylko dla TYPE_OBJECT i TYPE_ARRAY – dlaczego nie dla wszystkich wartości? Osobiście po prostu umieściłem link do json.org tuż przed enum. Twoje typy są analogiczne do te tam, musisz tylko udokumentować, co powinno być TYPE_KEY. To prowadzi mnie do następnego punktu …

TYPE_KEY

Spoglądając na json.org , widać, że obiekt składa się z lista członków , które z kolei składają się z ciągu znaków i wartości . Co oznacza, że tak naprawdę nie potrzebujesz TYPE_KEY! Po prostu dodaj nową strukturę dla członków składającą się z wartości TYPE_STRING i innej wartości JSON dowolnego typu i gotowe. W tej chwili możesz mieć np. liczbę jako klucz dla wartości, co jest niedozwolone. Ulepszyłoby też niektóre logiki związane z obiektami, na przykład pętla for:

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

Jak na ironię, krok pętli for faktycznie mógłby zawierać komentarz (dlaczego += 2?), Ale go brakuje.

Różne

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

Dlaczego nie tylko return 0;?


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

i

if (success) ++(*cursor); 

i

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

i kilka innych. Nie rozumiem szczególnie lubi umieszczać warunek i instrukcję w tej samej linii, zwłaszcza że nie robisz tego konsekwentnie.Nie mam nic przeciwko robieniu tego ze względu na przepływ kontroli, na przykład if (!something) return;, ale nadal jest to „meh”. Lepiej zrób to dobrze i umieść instrukcję w nowym wierszu.


Uważam również, że Twój kod może użyć kilku pustych wierszy do oddzielenia „regionów” lub jakkolwiek chcesz je nazwać . Na przykład:

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; 

Jest jedna pusta linia oddzielająca czynności związane z konfiguracją i analizą od czynności sprawdzania i zwrotu, ale możesz to zrobić lepiej.

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; 

Uważam, że jest to o wiele czystsze. Masz blok do ustawiania wartości, blok do ich analizowania, blok do ich wstawiania do wektora blok do pomijania białych znaków i blok do finalizowania bieżącej akcji. Ostatnia pusta linia między skip_whitespace(cursor); a if ... jest dyskusyjna , ale wolę to w ten sposób.


Poza tym stwierdziłem, że Twój kod jest czytelny i zrozumiały. Poprawnie sprawdzasz błędy i stosujesz rozsądne nazewnictwo. Co do idiomatyczności, poza z tego, o czym wspomniałem, nie ma niczego, co nazwałbym jako niezwykłe lub nie-idomatyczne.

Odpowiedź

Funkcje z ctype.h nie mogą być wywoływane z argumentami typu char, ponieważ może to wywołać niezdefiniowane zachowanie. Dobre wyjaśnienie można znaleźć w dokumentacji NetBSD .

Dodaj komentarz

Twój adres email nie zostanie opublikowany. Pola, których wypełnienie jest wymagane, są oznaczone symbolem *