En-tête

En C, tous les noms enum partagent le même espace de noms entre eux ( et avec des choses comme les noms de variables). Cest donc une bonne idée dessayer de réduire le risque de collision.

Vos noms enum json_value_type ont le préfixe TYPE_, ce qui est assez générique. Une autre bibliothèque peut essayer dutiliser le même nom. Je suggère de changer ce préfixe en, disons, JSON_.

De plus, vous ne semblez pas utiliser TYPE_KEY pour tout. Supprimez-le simplement.

Implémentation

Comme le note Roland Illig , les arguments de iscntrl() et isspace() dans votre fonction skip_whitespace() doivent être convertis en unsigned char en éviter lextension de signe.

Alternativement, et en suivant de plus près la spécification JSON , vous pouvez réécrire cette fonction simplement comme:

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

Un grand nombre de vos fonctions dassistance static font des combinaisons non triviales de choses, et ne contiennent aucun commentaire expliquant ce quelles fais. Une ou deux lignes de commentaires avant chaque fonction pourraient grandement améliorer la lisibilité.

En particulier, votre fonction has_char() fait un tas de choses différentes:

1. Il ignore les espaces.
2. Il vérifie la présence dun certain caractère dans lentrée.
3. Si le caractère est trouvé, il lignore automatiquement.

Seul # 2 est évidemment impliqué par le nom de la fonction; les autres sont des effets secondaires inattendus, et devraient au moins être clairement documentés.

En fait, il me semble quil serait peut-être préférable de supprimer lappel à skip_whitespace() à partir de has_char(), et laissez simplement lappelant sauter explicitement les espaces avant de lappeler si nécessaire. Dans de nombreux cas, votre code le fait déjà, ce qui rend le saut en double redondant.

De plus, pour rendre leffet n ° 3 moins surprenant pour le lecteur, il peut être judicieux de renommer cette fonction en quelque chose dun peu plus actif comme, par exemple, read_char().

À la fin de json_parse_object(), vous avez:

 return success; return 1; } 

C’est sûrement redondant. Débarrassez-vous simplement de return 1;.

Aussi, il semble que vous « utilisez la fonction générique json_parse_value() pour analyser les clés dobjet, et ne faites pas de test pour vous assurer quil sagit de chaînes. Cela permet à certains JSON non valides de passer par votre analyseur. Je suggère dajouter une vérification de type explicite ou de diviser votre code danalyse de chaîne en une fonction distincte (comme décrit ci-dessous) et de lappeler directement depuis json_parse_object().

En haut de json_parse_array(), vous avez:

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

Vous pouvez réécrire cela de la même manière que vous faites dans json_parse_object():

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

(Seulement, vous savez, je pense toujours que le nom read_char() serait mieux.)

De plus, pour une raison quelconque, votre json_parse_array() semble sattendre à ce que lappelant initialise le parent struct, alors que json_parse_object() le fait automatiquement. AFAICT il ny a aucune raison pour cette incohérence, donc vous pourriez et devriez probablement faire fonctionner les deux fonctions de la même manière.

Votre fonction json_is_literal() nest pas marquée comme static, même si ce nest pas le cas apparaissent dans len-tête. Comme is_char(), I « d al préférez donc le renommer en quelque chose de plus actif, comme json_read_literal() ou simplement read_literal(), pour quil soit plus clair quil avance automatiquement le curseur sur un correspondance réussie.

(Notez également que, telle quelle est écrite, cette fonction ne vérifie pas que le littéral dans lentrée se termine réellement là où il est supposé. Par exemple, il correspondrait avec succès à lentrée nullnullnull et null.Je ne pense pas que ce soit un bogue réel, puisque les seuls littéraux valides dans JSON sont true, false et null, dont aucun nest un préfixe lun de lautre, et puisque deux littéraux ne peuvent pas apparaître consécutivement dans un JSON valide sans un autre jeton entre les deux. Mais cela vaut au moins la peine dêtre noté dans un commentaire.)

Vous pouvez également marquer explicitement certaines de vos fonctions dassistance statiques comme inline pour donner au compilateur une indication quil devrait essayer de les fusionner dans le code appelant. Je « suggérerais de le faire au moins pour skip_whitespace(), has_char() et json_is_literal().

Puisque vos fonctions daccès json_value_to_X() ne consistent en rien dautre quun assert() et une déréférence de pointeur, vous devriez également envisager de déplacer leurs implémentations dans json.h et de les marquer comme static inline. Cela permettrait au compilateur de les intégrer dans le code appelant même dans dautres fichiers .c, et éventuellement doptimiser les assert() si lappel code vérifie déjà le type de toute façon.

Dans votre fonction principale json_parse(), vous voudrez peut-être vérifier explicitement quil ne reste que des espaces dans le entrée après que la valeur racine a été analysée.

Analyse de chaîne

Votre code danalyse de chaîne dans json_parse_value() est cassé, car il ne « t gérer les échappements de backslash. Par exemple, il échoue sur lentrée JSON valide suivante:

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

Vous pouvez ajouter cela comme cas de test.

Vous doit également vérifier que votre code gère correctement les autres séquences déchappement de barre oblique inverse telles que \b, \f, \n, \r, \t, \/ et en particulier \\ et \unnnn. Voici quelques cas de test supplémentaires pour ceux-ci:

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

Puisque les chaînes JSON peuvent contenir des caractères Unicode arbitraires, vous devrez décider comment les gérer. Le choix le plus simple serait probablement de déclarer vos entrées et sorties en UTF-8 (ou peut-être WTF-8 ) et de convertir \unnnn séchappe dans des séquences doctets UTF-8 (et, facultativement, vice versa). Notez que, puisque vous « utilisez des chaînes terminées par NULL, vous préférerez peut-être décoder \u0000 en le codage trop long "\xC0\x80" au lieu dun octet nul.

Afin de garder la fonction principale json_parse_value() lisible, Je recommanderais fortement de diviser le code danalyse de chaîne en une fonction dassistance distincte. Surtout que le faire gérer correctement les échappements de contre-oblique compliquera considérablement la tâche.

Lune des complications est que vous ne saurez pas pendant combien de temps le la chaîne sera jusquà ce que vous layez analysée. Une façon de gérer cela serait de développer dynamiquement la chaîne de sortie allouée avec realloc(), par exemple comme ceci:

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

Une autre solution serait dexécuter deux passes danalyse sur lentrée: une juste pour déterminer la longueur de la chaîne de sortie, et une autre pour la décoder. Ce serait plus facile à faire quelque chose comme ça:

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

où la fonction dassistance:

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

analyse une chaîne JSON dau plus *length octets dans new_string et écrit la longueur réelle de la chaîne analysée dans *length, ou, if new_string == NULL, détermine simplement la longueur de la chaîne sans réellement stocker la sortie décodée nimporte où.

Analyse des nombres

Votre json_parse_value() limplémentation traite les nombres comme cas par défaut, et alimente simplement tout ce qui nest pas avec ", [, {, n, t ou f dans la fonction de bibliothèque standard C strtod().

Puisque strtod() accepte un sur-ensemble de littéraux JSON valides, cela devrait fonctionner, mais peut parfois rendre votre code ac cept JSON invalide comme valide. Par exemple, votre code acceptera +nan, -nan, +inf et -inf comme des nombres valides, et acceptera également la notation hexadécimale comme 0xABC123. De plus, comme le note la documentation de strtod() ci-dessus:

Dans une langue autre que le standard « C » ou « POSIX » locales, cette fonction peut reconnaître une syntaxe supplémentaire dépendante des paramètres régionaux.

Si vous voulez être plus strict, vous voudrez peut-être valider explicitement tout ce qui ressemble à un nombre par rapport à grammaire JSON avant de la transmettre à strtod().

Notez également que strtod() peut définir errno par exemple si le numéro dentrée est en dehors de la plage de double. Vous devriez probablement vérifier cela.

Test

Je nai pas examiné vos tests en détail, mais cest formidable de voir que vous les avez (même si, comme indiqué ci-dessus, leur couverture pourrait être améliorée).

Personnellement, cependant, je préfère déplacer les tests de limplémentation dans un fichier source séparé. Cela présente à la fois des avantages et des inconvénients:

• Le principal inconvénient est que vous ne pouvez plus tester directement les fonctions dassistance statiques. Cependant, étant donné que votre API publique a lair propre et complète, et ne souffre pas de problèmes d « état caché » qui compliqueraient les tests, vous devriez être en mesure dobtenir une bonne couverture de test même uniquement via lAPI.
• Le principal avantage (outre une séparation nette entre le code dimplémentation et le code de test) est que vos tests testeront automatiquement lAPI publique. En particulier, tout problème avec len-tête json.h apparaîtra dans vos tests. De plus, faire vos tests via lAPI vous permet de vous assurer que votre API est vraiment suffisamment complète et flexible pour une utilisation générale.

Si vous voulez vraiment tester directement vos fonctions statiques, vous pouvez toujours ajouter un indicateur de préprocesseur qui les expose éventuellement pour les tests, soit via de simples wrappers, soit simplement en supprimant le mot-clé static de leurs définitions.

Ps. I a remarqué que votre test json_test_value_number() échoue pour moi (GCC 5.4.0, i386 arch), vraisemblablement car le nombre 23,4 nest pas exactement représentable en virgule flottante. Le changer en 23,5 rend le test réussi.

Commentaires

• Excellent travail, merci, évidemment je nai pas ‘ t vérifier la norme autant que jaurais dû. Bien que jaie volontairement ignoré utf-8 (jaurais probablement dû le mentionner.
• OK, cest juste. Vous devriez probablement encore au moins décoder de simples échappements de la forme \u00XX, puisque certains encodeurs peuvent choisir de les utiliser même pour les caractères ASCII. (Jai également ajouté une suggestion pour marquer certaines de vos fonctions comme inline ci-dessus, car jai oublié de le faire plus tôt .)
• Jai totalement manqué cette partie du standard, oui les séquences déchappement doivent être analysées correctement.

Ce nest en aucun cas une critique complète, mais je « partagerai certaines choses qui ont attiré mon attention en lisant votre code.

Commentaires

Bien les commentaires sont sûrement sympas, certains de vos commentaires intégrés najoutent que du bruit au code.

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

Tout dabord, le commentaire est une ligne trop tôt. Deuxièmement, on peut lire que lespace blanc est consommé en regardant la fonction – le nom la décrit parfaitement, e Il ny a pas besoin de commentaire supplémentaire.

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

Encore une fois, ce commentaire ne fait que répéter ce que le code lui-même dit.

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

Maintenant, ces commentaires ne sont pas vraiment inutiles puisquils documentent ce que chaque valeur représente. Mais pourquoi seulement pour TYPE_OBJECT et TYPE_ARRAY – pourquoi pas pour toutes les valeurs? Personnellement, je « mettrais juste un lien vers json.org juste avant enum. Vos types sont analogues à ceux qui sont là, il vous suffit de documenter ce que TYPE_KEY est censé être. Ce qui mamène au point suivant …

TYPE_KEY

En regardant json.org , vous pouvez voir quun objet se compose de une liste de membres , qui à leur tour sont constitués dune chaîne et dune valeur . Cela signifie que vous navez pas vraiment besoin de TYPE_KEY! Ajoutez simplement une nouvelle structure pour membres consistant en une valeur TYPE_STRING et une autre valeur json de nimporte quel type et vous êtes prêt à partir. Pour le moment, vous pouvez avoir par exemple un nombre comme clé pour une valeur, ce qui nest pas autorisé. Cela rendrait aussi une partie de la logique liée à lobjet plus agréable, comme ceci pour boucle:

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

Ironiquement, létape de cette boucle for pourrait en fait utiliser un commentaire (pourquoi += 2?) Mais il en manque un.

Divers

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

Pourquoi pas simplement return 0;?

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

et

if (success) ++(*cursor); 

et

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

et quelques autres. Je « ne suis pas aime particulièrement mettre la condition et la déclaration sur la même ligne, dautant plus que vous ne le faites pas systématiquement.Je « suis plutôt daccord avec le fait de faire cela pour des raisons de contrôle du flux, comme if (!something) return;, mais cest toujours » meh « . Mieux vaut le faire correctement et mettre la déclaration sur une nouvelle ligne.

De plus, je trouve que votre code pourrait utiliser des lignes plus vides pour séparer les « régions » ou tout ce que vous aimeriez les appeler . Par exemple:

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; 

Il y a une ligne vide séparant les éléments setup-and-parse-stuff du check-and-return, mais vous pouvez le faire mieux.

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; 

Je trouve que cest beaucoup plus propre. Vous avez un bloc pour configurer les valeurs, un bloc pour les analyser, un bloc pour les mettre dans le vecteur, un bloc pour sauter les espaces et un bloc pour finaliser laction en cours. La dernière ligne vide entre skip_whitespace(cursor); et if ... est discutable , mais je préfère que ce soit ainsi.

À part cela, jai trouvé que votre code était facilement lisible et compréhensible. Vous vérifiez correctement les erreurs et utilisez une dénomination judicieuse. Quant à lidiomaticité, à part daprès ce que jai mentionné, il ny a rien que je puisse marquer comme inhabituel ou non idomatique.

Les fonctions de ctype.h ne doivent pas être appelées avec des arguments de type char, car cela peut invoquer un comportement indéfini. Consultez la documentation NetBSD pour une bonne explication.