Re: [PATCH] block: partitions: Replace pp_buf with struct seq_buf

[David Laight <david.laight.linux@gmail.com>]

On Mon, 23 Mar 2026 10:51:44 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Fri, Mar 20, 2026 at 05:48:44PM -0700, Kees Cook wrote:
> > In preparation for removing the strlcat API[1], replace the char *pp_buf
> > with a struct seq_buf, which tracks the current write position and
> > remaining space internally. This allows for:
> > 
> > - Direct use of seq_buf_printf() in place of snprintf()+strlcat()
> >   pairs, eliminating local tmp buffers throughout.
> > - Adjacent strlcat() calls that build strings piece-by-piece
> >   (e.g., strlcat("["); strlcat(name); strlcat("]")) to be collapsed
> >   into single seq_buf_printf() calls.
> > - Simpler call sites: seq_buf_puts() takes only the buffer and string,
> >   with no need to pass PAGE_SIZE at every call.
> > 
> > The backing buffer allocation is unchanged (__get_free_page), and the
> > output path uses seq_buf_str() to NUL-terminate before passing to
> > printk().  
> 
> Thanks a lot! A few comments below.
> Personally I'm in favour of this patch as it also removes a lot of ugly code
> (which is scoped string manipulations), FWIW,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> > Link: https://github.com/KSPP/linux/issues/370 [1]  
> 
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Josh Law <objecting@objecting.org>  
> 
> While not long, this still can be placed...
> 
> > Signed-off-by: Kees Cook <kees@kernel.org>
> > ---  
> 
> ...somewhere here to reduce unneeded noise in the commit message.
> 
> > I couldn't help myself. Here's the full patch, as I suggested in
> > https://lore.kernel.org/lkml/202603201230.74BBFFABAD@keescook/
> > There are plenty more like this to do...  
> 
> Indeed, but thanks for the example on how to do that!
> 
...
> But probably okay as in the previous branch it needs more work to follow,
> something like
> 
> 			char dostype[8];
> 			...
> 			if (dostype[3] < ' ') {
> 				/* Escape control character */
> 				dostype[4] = dostype[3] + '@';
> 				dostype[3] = '^';
> 				seq_buf_printf(&state->pp_buf, " (%.5s)", dostype);

Or just:
				seq_buf_printf(&state->pp_buf, " (%.3s^%c)", dostype, dostype[3] + '@');

	David

> 			} else {
> 				seq_buf_printf(&state->pp_buf, " (%.4s)", dostype);
> 			}
> 
> Taking how invasive is this, it might be better to done separately.
> 
> > +			seq_buf_printf(&state->pp_buf, "(res %d spb %d)",
> > +				       be32_to_cpu(pb->pb_Environment[6]),
> > +				       be32_to_cpu(pb->pb_Environment[4]));
> >  		}
> >  		res = 1;  
>