Re: [PATCH 1/4] version: refactor redact_non_printables()
From: Eric Sunshine <hidden>
Date: 2025-01-06 22:35:22
On Mon, Jan 6, 2025 at 5:37 AM Usman Akinyemi [off-list ref] wrote:
The git_user_agent_sanitized() function performs some sanitizing to avoid special characters being sent over the line and possibly messing up with the protocol or with the parsing on the other side. Let's extract this sanitizing into a new redact_non_printables() function, as we will want to reuse it in a following patch. For now the new redact_non_printables() function is still static as it's only needed locally. While at it, let's also make a few small improvements: - use 'size_t' for 'i' instead of 'int', - move the declaration of 'i' inside the 'for ( ... )',
Regarding the above two items...
quoted hunk ↗ jump to hunk
- use strbuf_detach() to explicitly detach the string contained by the 'buf' strbuf. Mentored-by: Christian Couder [off-list ref] Signed-off-by: Usman Akinyemi <redacted> ---diff --git a/version.c b/version.c@@ -6,6 +6,20 @@ +static void redact_non_printables(struct strbuf *buf) +{ + strbuf_trim(buf); + for (size_t i = 0; i < buf->len; i++) { + if (buf->buf[i] <= 32 || buf->buf[i] >= 127) + buf->buf[i] = '.'; + } +}@@ -27,12 +41,8 @@ const char *git_user_agent_sanitized(void) strbuf_addstr(&buf, git_user_agent()); - strbuf_trim(&buf); - for (size_t i = 0; i < buf.len; i++) {
... the original code appears to have already been using `size_t` and declaring the loop variable inside the `for` statement, despite what the commit message says. So, is the commit message out of date? Or are the patches out of order? Or something else?
- if (buf.buf[i] <= 32 || buf.buf[i] >= 127) - buf.buf[i] = '.'; - } - agent = buf.buf; + redact_non_printables(&buf); + agent = strbuf_detach(&buf, NULL);