Re: [PATCH v2 1/2] lib: add sputchar() helper

From: Petr Mladek
Date: Tue Sep 07 2021 - 05:35:33 EST


On Mon 2021-09-06 13:51:59, Rasmus Villemoes wrote:
> On 05/09/2021 01.10, Yury Norov wrote:
> > There are 47 occurrences of the code snippet like this:
> > if (buf < end)
> > *buf = ' ';
> > ++buf;
> >
> > This patch adds a helper function sputchar() to replace opencoding.
> > It adds a lot to readability, and also saves 43 bytes of text on x86.
> >
> > v2: cleanup cases discovered with coccinelle script.
> >
> > Signed-off-by: Yury Norov <yury.norov@xxxxxxxxx>
> > ---
> > include/linux/kernel.h | 8 ++
>
> Sorry, but 47 cases, mostly in one .c file, is not enough justification
> for adding yet another piece of random code to
> the-dumping-ground-which-is-kernel.h, especially since this helper is
> very specific to the needs of the vsnprintf() implementation, so
> extremely unlikely to find other users.

What about putting it into include/linux/string_helpers.h ?

Or create include/linux/vsprintf.h ?

> I'm also not a fan of the sputchar name - it's unreadable at first
> glance, and when you figure out it's "a _s_tring version of putchar",
> that doesn't help, because its semantics are nothing like the stdio putchar.

I do not like the name either.

What about vsprintf_putc(buf, end, c) or vsp_putc(buf, end, c)?

Well, the ugly thing are the three parameters. Which brings an idea of

struct vsprintf_buffer { // or struct vsp_buf
char *buf,
char *end,
}

and using vprintf_putc(vsp_buf, c) or vsp_putc(vsp_buf, c).

The change would look like: