On 9/19/23 1:48 PM, Taylor Blau wrote:
[+cc Jeff Hostetler]
On Tue, Sep 19, 2023 at 11:54:58AM +0000, mark via GitGitGadget wrote:
diff --git a/json-writer.c b/json-writer.c
index 005c820aa42..23ba7046e5d 100644
--- a/json-writer.c
+++ b/json-writer.c
@@ -20,6 +20,11 @@ static void append_quoted_string(struct strbuf *out, const char *in)
{
unsigned char c;
+ if (!in || !*in) {
+ strbuf_addstr(out, "\"\"");
+ return;
+ }
From reading the implementation of append_quoted_string(), I think that
the case where "in" is the empty string is already covered. IOW, doing
something like:
struct strbuf buf = STRBUF_INIT;
append_quoted_string(&out, "");
warning("'%s'", buf.buf);
would print out something like:
warning: '""'
as expected. Handling a NULL "in" argument is new behavior, but I am not
sure if it is appropriate to coerce a NULL input into the empty string.
I've CC'd the author of this code, whose opinion I trust more than my
own here.
Thanks,
Taylor
There are three callers of `append_quoted_string()` and it is static
to the json-writer.c code.
Basically, in a JSON object, we have 2 uses:
{
"<key>" : "<string-value>",
"<key>" : <integer>,
...
}
And in a JSON array, we have the other:
[
"<string-value>",
...
]
I suppose it is OK for the 2 string-value cases to assume a NULL pointer
could be written as "" in the JSON output. Although, I kinda think a
NULL pointer should call BUG() as we have in the various assert_*()
routines. It really is a kind of logic error in the caller.
Regardless what we decide for the <string-value> case, in the <key>
case, the resulting JSON would not be valid. We need for the key to
be a non-empty string. For example { "" : 1 } is not valid JSON.
So the key case should call BUG() and not try to hide it.
So I'm leaning towards just making it a BUG() in all cases, but I'm
open to the other mixed handling.
Jeff
