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

[Kees Cook]

On Mon, Mar 23, 2026 at 08:08:07PM +0000, David Laight wrote:
> On Mon, 23 Mar 2026 10:51:44 +0200
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> > 
> > > Link: https://github.com/KSPP/linux/issues/370 [1]  
> > 
> > > Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > > Cc: Josh Law <objecting@xxxxxxxxxxxxx>  
> > 
> > While not long, this still can be placed...
> > 
> > > Signed-off-by: Kees Cook <kees@xxxxxxxxxx>
> > > ---  
> > 
> > ...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.

I tried to make this as 1-to-1 replacement as possible. The logic here
was "complex" enough that I just left it as-is.


-- 
Kees Cook