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

[Petr Mladek]

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: