Re: [PATCH v3 1/5] version: refactor strbuf_sanitize()
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Jan 27, 2025, 15:07 UTC
- Message-ID
- <CAP8UFD2b71dGmqBh5Px4F9E6Jetdo5FdzE9GKP2CsLVWXexHUQ@mail.gmail.com>
- In-Reply-To
- <xmqqfrn0ui7f.fsf@gitster.g>
On Sat, Dec 7, 2024 at 7:21 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 16 quoted lines
> > Christian Couder <christian.couder@gmail.com> writes: > > > +/* > > + * Trim and replace each character with ascii code below 32 or above > > + * 127 (included) using a dot '.' character. Useful for sending > > + * capabilities. > > + */ > > +void strbuf_sanitize(struct strbuf *sb); > > I am not getting "Useful for sending capabilities" here, and feel > that it is somewhat an unsubstantiated claim. If some information > is going to be transferred (which the phrase "sending capabilities" > hints), I'd expect that we try as hard as possible not to lose > information, but redact-non-ASCII is the total opposite of "not > losing information".
Ok, "Useful for sending capabilities" will be removed.
Show 26 quoted lines
> By the way, as we are trimming, I am very very much tempted to
> squish a run of non-ASCII bytes into one dot, perhaps like
>
> void redact_non_printables(struct strbuf *sb)
> {
> size_t dst = 0;
> int skipped = 0;
>
> strbuf_trim(sb);
> for (size_t src = 0; src < sb->len; src++) {
> int ch = sb->buf[src];
> if (ch <= 32 && 127 <= ch) {
> if (skipped)
> continue;
> ch = '.';
> }
> sb->buf[dst++] = ch;
> skipped = (ch == '.');
> }
> }
>
> or even without strbuf_trim(), which would turn any leading or
> trailing run of whitespaces into '.'.
>
> But that is an improvement that can be easily done on top after the
> dust settles and better left as #leftoverbits material.Usman's patch series about introducing a "os-version" capability needs such a feature too, and Usman already reworked this code according to your comments here. It looks like you found it good too. So I will just reuse his patches related to this in the version 4 of this patch series.