Re: [PATCH 06/15] strbuf: accept a comment string for strbuf_stripspace()
- From
Jeff King <peff@peff.net>
- Date
- Mar 7, 2024, 09:53 UTC
- Message-ID
- <20240307095332.GB2650063@coredump.intra.peff.net>
- In-Reply-To
- <20240307092126.GF2080210@coredump.intra.peff.net>
On Thu, Mar 07, 2024 at 04:21:26AM -0500, Jeff King wrote:
Show 8 quoted lines
> As part of our transition to multi-byte comment characters, let's take a > NUL-terminated string pointer for strbuf_stripspace(), rather than a > single character. We can continue to support its feature of ignoring > comments by accepting a NULL pointer (as opposed to the current behavior > of a NUL byte). > > All of the callers have to be adjusted, but they can all just pass > comment_line_str (or NULL).
Bah. I relied on the compiler to tell me the call-sites that needed to be adjusted. But interestingly gcc is quite happy to allow '\0' to be passed in place of a pointer, but clang complains:
gpg-interface.c:589:37: error: expression which evaluates to zero treated as a null pointer constant of type 'const char *' [-Werror,-Wnon-literal-null-conversion]
strbuf_stripspace(&ssh_keygen_out, '\0');
^~~~Likewise there are a few bare "0"'s which do not cause a warning, but which violate our style standards. So I think we'd want to squash the patch below in to this step. The other functions don't need the same treatment because they never treated NUL specially.
---
diff --git a/builtin/am.c b/builtin/am.c index d1990d7edc..5bc72d7822 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1286,7 +1286,7 @@ static int parse_mail(struct am_state *state, const char *mail) strbuf_addstr(&msg, "\n\n"); strbuf_addbuf(&msg, &mi.log_message); - strbuf_stripspace(&msg, '\0'); + strbuf_stripspace(&msg, NULL); assert(!state->author_name); state->author_name = strbuf_detach(&author_name, NULL); diff --git a/builtin/commit.c b/builtin/commit.c index 8519a004d0..e04f1236e8 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -890,7 +890,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, s->hints = 0; if (clean_message_contents) - strbuf_stripspace(&sb, '\0'); + strbuf_stripspace(&sb, NULL); if (signoff) append_signoff(&sb, ignored_log_message_bytes(sb.buf, sb.len), 0); diff --git a/builtin/notes.c b/builtin/notes.c index 1a67f01d00..cb011303e6 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -264,7 +264,7 @@ static void concat_messages(struct note_data *d) if ((d->stripspace == UNSPECIFIED && d->messages[i]->stripspace == STRIPSPACE) || d->stripspace == STRIPSPACE) - strbuf_stripspace(&d->buf, 0); + strbuf_stripspace(&d->buf, NULL); strbuf_reset(&msg); } strbuf_release(&msg); diff --git a/builtin/worktree.c b/builtin/worktree.c index 9c76b62b02..f0aa962cf8 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -657,7 +657,7 @@ static int can_use_local_refs(const struct add_opts *opts) strbuf_add_real_path(&path, get_worktree_git_dir(NULL)); strbuf_addstr(&path, "/HEAD"); strbuf_read_file(&contents, path.buf, 64); - strbuf_stripspace(&contents, 0); + strbuf_stripspace(&contents, NULL); strbuf_strip_suffix(&contents, "\n"); warning(_("HEAD points to an invalid (or orphaned) reference.\n" diff --git a/gpg-interface.c b/gpg-interface.c index 95e764acb1..b5993385ff 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -586,8 +586,8 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc, } } - strbuf_stripspace(&ssh_keygen_out, '\0'); - strbuf_stripspace(&ssh_keygen_err, '\0'); + strbuf_stripspace(&ssh_keygen_out, NULL); + strbuf_stripspace(&ssh_keygen_err, NULL); /* Add stderr outputs to show the user actual ssh-keygen errors */ strbuf_add(&ssh_keygen_out, ssh_principals_err.buf, ssh_principals_err.len); strbuf_add(&ssh_keygen_out, ssh_keygen_err.buf, ssh_keygen_err.len);