On 6/15/26 19:01, Markus Armbruster wrote:
Looks pretty good. One more respin should get us there.
Cool, I attach the current overall delta from your review comments.
Paolo
diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 8c58ae0349a..a188d58d006 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -71,9 +71,12 @@ typedef struct JSONParserStackEntry {
* description, "-> action" represents the pseudocode of the action
* and "-> STATE" sets the top stack entry's state to STATE.
*
+ * The state alone is enough to tell you what to parse; the state plus
+ * the type of the top of stack tells you which action to take.
+ *
* // The initial state is BEFORE_VALUE.
* input := value -> END_OF_VALUE -> return parsed value
- * END_OF_INPUT -> check stack is empty
+ * (input | END_OF_INPUT)
*
* // entered on BEFORE_VALUE; after any of these rules are processed, the
* // parser has completed a QObject and is in the END_OF_VALUE state.
@@ -122,6 +125,9 @@ typedef struct JSONParserStackEntry {
* ('}' -> pop completed QDict -> END_OF_VALUE
* | ',' -> BEFORE_KEY
* dict_pairs) -> END_OF_VALUE
+ *
+ * Parse errors ignore the token. json_parser_reset() can be
+ * called to restart parsing from scratch, with an empty stack.
*/
#define BUG_ON(cond) assert(!(cond))
@@ -140,6 +146,7 @@ static void push_entry(JSONParserContext *ctxt, QObject *partial,
g_queue_push_tail(ctxt->stack, entry);
}
+/* Drop the top entry and return the new top entry. */
static JSONParserStackEntry *pop_entry(JSONParserContext *ctxt)
{
JSONParserStackEntry *entry = g_queue_pop_tail(ctxt->stack);
@@ -163,7 +170,7 @@ static void G_GNUC_PRINTF(3, 4) parse_error(JSONParserContext *ctxt,
va_start(ap, msg);
vsnprintf(message, sizeof(message), msg, ap);
va_end(ap);
- error_setg(&ctxt->err, "JSON parse error at line %d, column %d, %s",
+ error_setg(&ctxt->err, "%d:%d: JSON parse error, %s",
token->y, token->x, message);
}
@@ -483,20 +490,21 @@ static QObject *parse_token(JSONParserContext *ctxt, const JSONToken *token)
case BEFORE_KEY:
/* Expecting object key */
assert(qobject_type(entry->partial) == QTYPE_QDICT);
- if (token->type == JSON_STRING || token->type == JSON_INTERP) {
- key_obj = parse_begin_value(ctxt, token);
- if (!key_obj) {
- /* parse error happened */
- return NULL;
- }
- }
- if (!key_obj || qobject_type(key_obj) != QTYPE_QSTRING) {
- parse_error(ctxt, token, "key is not a string in object");
+ if (token->type != JSON_STRING && token->type != JSON_INTERP) {
+ parse_error(ctxt, token, "expecting value");
return NULL;
}
- /* Store key in a special entry on the stack */
- push_entry(ctxt, key_obj, END_OF_KEY);
+ key_obj = parse_begin_value(ctxt, token);
+ if (!key_obj) {
+ /* Parse error already reported */
+ } else if (qobject_type(key_obj) != QTYPE_QSTRING) {
+ /* An interpolation was valid syntactically but not %s */
+ parse_error(ctxt, token, "key is not a string in object");
+ } else {
+ /* Store key in a special entry on the stack */
+ push_entry(ctxt, key_obj, END_OF_KEY);
+ }
return NULL;
case END_OF_KEY:
@@ -565,15 +573,21 @@ static QObject *parse_token(JSONParserContext *ctxt, const JSONToken *token)
assert(value);
if (entry == NULL) {
- /* The toplevel value is complete. */
+ /* Parse stack now empty, the top-level value is complete. */
return value;
}
+ /*
+ * Parse stack is not empty and entry->partial is the top of stack.
+ * It's a QString with the key (and a QDict is below it) if we're
+ * parsing an object, or a QList if we're parsing an array.
+ */
key = qobject_to(QString, entry->partial);
if (key) {
const char *key_str;
QDict *dict;
+ /* Pop off key, and store (key, value) in QDict. */
entry = pop_entry(ctxt);
dict = qobject_to(QDict, entry->partial);
assert(dict);
@@ -586,7 +600,7 @@ static QObject *parse_token(JSONParserContext *ctxt, const JSONToken *token)
qdict_put_obj(dict, key_str, value);
qobject_unref(key);
} else {
- /* Add to array */
+ /* Array, just store value in the QList. */
qlist_append_obj(qobject_to(QList, entry->partial), value);
}
@@ -622,7 +636,7 @@ void json_parser_destroy(JSONParserContext *ctxt)
/*
* Advance the parser based on the token that is passed.
- * Return the finished toplevel value if the token completes it.
+ * Return the finished top-level value if the token completes it.
* If an error is returned, the function must not be called without
* first resetting the parser.
*/
@@ -634,7 +648,7 @@ QObject *json_parser_feed(JSONParserContext *ctxt, const JSONToken *token,
assert(!ctxt->err);
switch (token->type) {
case JSON_ERROR:
- parse_error(ctxt, token, "JSON parse error, stray '%s'", token->str);
+ parse_error(ctxt, token, "stray '%s'", token->str);
break;
case JSON_END_OF_INPUT:
diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c
index 2c1f20fc62b..6d7c947f94a 100644
--- a/qobject/json-streamer.c
+++ b/qobject/json-streamer.c
@@ -34,7 +34,7 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
parser->brace_count++;
break;
case JSON_RCURLY:
- if (parser->brace_count <= 0) {
+ if (!parser->brace_count) {
goto end_error_recovery;
}
parser->brace_count--;
@@ -43,7 +43,7 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
parser->bracket_count++;
break;
case JSON_RSQUARE:
- if (parser->bracket_count <= 0) {
+ if (!parser->bracket_count) {
goto end_error_recovery;
}
parser->bracket_count--;
@@ -51,9 +51,10 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
case JSON_ERROR:
end_error_recovery:
/*
- * Cause error recovery to end immediately.
- * If not in error recovery, the parser will raise an error
- * (due to JSON_ERROR or unexpected JSON_R{CURLY,SQUARE})
+ * We come here due to receiving either JSON_ERROR or a
+ * JSON_R{CURLY,SQUARE}) that is known to be unbalanced.
+ * If in error recovery, end it immediately. If not in
+ * error recovery, json_parser_feed() will raise an error
* but error recovery won't be entered at all.
*/
parser->brace_count = 0;
@@ -98,12 +99,12 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
if ((parser->brace_count == 0 && parser->bracket_count == 0)
|| type == JSON_END_OF_INPUT) {
+ json_parser_reset(&parser->parser);
parser->error = false;
parser->brace_count = 0;
parser->bracket_count = 0;
parser->token_count = 0;
parser->token_size = 0;
- json_parser_reset(&parser->parser);
}
}
