Re: [PATCH 06/15] strbuf: accept a comment string for strbuf_stripspace()

[Jeff King <peff@peff.net>]

On Thu, Mar 07, 2024 at 04:21:26AM -0500, Jeff King wrote:

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