diff mbox series

[v4,3/6] version: make redact_non_printables() non-static

Message ID 20250127151701.2321341-4-christian.couder@gmail.com (mailing list archive)
State Superseded
Headers show
Series Introduce a "promisor-remote" capability | expand

Commit Message

Christian Couder Jan. 27, 2025, 3:16 p.m. UTC 
As we are going to reuse redact_non_printables() outside "version.c",
let's make it non-static.
---
 version.c | 6 +-----
 version.h | 8 ++++++++
 2 files changed, 9 insertions(+), 5 deletions(-)

Comments

Patrick Steinhardt Jan. 30, 2025, 10:51 a.m. UTC | #1 
On Mon, Jan 27, 2025 at 04:16:58PM +0100, Christian Couder wrote:
> As we are going to reuse redact_non_printables() outside "version.c",
> let's make it non-static.

Missing the DCO.

> diff --git a/version.h b/version.h
> index 7c62e80577..fcc1816685 100644
> --- a/version.h
> +++ b/version.h
> @@ -4,7 +4,15 @@
>  extern const char git_version_string[];
>  extern const char git_built_from_commit_string[];
>  
> +struct strbuf;
> +
>  const char *git_user_agent(void);
>  const char *git_user_agent_sanitized(void);
>  
> +/*
> + * Trim and replace each character with ascii code below 32 or above
> + * 127 (included) using a dot '.' character.
> +*/
> +void redact_non_printables(struct strbuf *buf);

Is this header really the right spot though? If I want to redact
characters I certainly wouldn't be looking at "version.h" for that
functionality.

Patrick
Christian Couder Feb. 18, 2025, 11:42 a.m. UTC | #2 
On Thu, Jan 30, 2025 at 11:51 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Mon, Jan 27, 2025 at 04:16:58PM +0100, Christian Couder wrote:
> > As we are going to reuse redact_non_printables() outside "version.c",
> > let's make it non-static.
>
> Missing the DCO.

Thanks for spotting this.

> > diff --git a/version.h b/version.h
> > index 7c62e80577..fcc1816685 100644
> > --- a/version.h
> > +++ b/version.h
> > @@ -4,7 +4,15 @@
> >  extern const char git_version_string[];
> >  extern const char git_built_from_commit_string[];
> >
> > +struct strbuf;
> > +
> >  const char *git_user_agent(void);
> >  const char *git_user_agent_sanitized(void);
> >
> > +/*
> > + * Trim and replace each character with ascii code below 32 or above
> > + * 127 (included) using a dot '.' character.
> > +*/
> > +void redact_non_printables(struct strbuf *buf);
>
> Is this header really the right spot though? If I want to redact
> characters I certainly wouldn't be looking at "version.h" for that
> functionality.

In previous versions of this series, I wanted to put this in the
strbuf API but it appeared not to be a good idea.

Anyway, now I think that this patch is not needed, thanks to a comment
you made about the following patch. So we don't need to find a good
place for it for now.
diff mbox series

Patch

diff --git a/version.c b/version.c
index 4f37b4499d..77423fcaf3 100644
--- a/version.c
+++ b/version.c
@@ -12,11 +12,7 @@ 
 const char git_version_string[] = GIT_VERSION;
 const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT;
 
-/*
- * Trim and replace each character with ascii code below 32 or above
- * 127 (included) using a dot '.' character.
- */
-static void redact_non_printables(struct strbuf *buf)
+void redact_non_printables(struct strbuf *buf)
 {
 	strbuf_trim(buf);
 	for (size_t i = 0; i < buf->len; i++) {
diff --git a/version.h b/version.h
index 7c62e80577..fcc1816685 100644
--- a/version.h
+++ b/version.h
@@ -4,7 +4,15 @@ 
 extern const char git_version_string[];
 extern const char git_built_from_commit_string[];
 
+struct strbuf;
+
 const char *git_user_agent(void);
 const char *git_user_agent_sanitized(void);
 
+/*
+ * Trim and replace each character with ascii code below 32 or above
+ * 127 (included) using a dot '.' character.
+*/
+void redact_non_printables(struct strbuf *buf);
+
 #endif /* VERSION_H */