On Wed, Jan 10, 2024 at 03:48:43PM -0800, Kees Cook wrote: > On Wed, Jan 10, 2024 at 02:36:30PM -0500, Kent Overstreet wrote: > > [...] > > bcachefs: %pg is banished > > Hi! > > Not a PR blocker, but this patch re-introduces users of strlcpy() which > has been otherwise removed this cycle. I'll send a patch to replace > these new uses, but process-wise, I'd like check on how bcachefs patches > are reviewed. I'm happy to fix it. Perhaps the declaration could get a depracated warning, though? > Normally I'd go find the original email that posted the patch and reply > there, but I couldn't find a development list where this patch was > posted. Where is this happening? (Being posted somewhere is supposed > to be a prerequisite for living in -next. E.g. quoting from the -next > inclusion boiler-plate: "* posted to the relevant mailing list,") It > looks like it was authored 5 days ago, which is cutting it awfully close > to the merge window opening: > > AuthorDate: Fri Jan 5 11:58:50 2024 -0500 I'm confident in my testing; if it was a patch that needed more soak time it would have waited. > Actually, it looks like you rebased onto v6.7-rc7? This is normally > strongly discouraged. The common merge base is -rc2. Is there something special about rc2? I reorder patches fairly often just in the normal course of backporting fixes, and if I have to rebase everything for a backport I'll often rebase onto a newer kernel so that the people who are running my tree are testing something more stable - it does come up. > It also seems it didn't get a run through scripts/checkpatch.pl, which > shows 4 warnings, 2 or which point out the strlcpy deprecation: > > WARNING: Prefer strscpy over strlcpy - see: https://github.com/KSPP/linux/issues/89 > #123: FILE: fs/bcachefs/super.c:1389: > + strlcpy(c->name, name.buf, sizeof(c->name)); > > WARNING: Prefer strscpy over strlcpy - see: https://github.com/KSPP/linux/issues/89 > #124: FILE: fs/bcachefs/super.c:1390: > + strlcpy(ca->name, name.buf, sizeof(ca->name)); > > Please make sure you're running checkpatch.pl -- it'll make integration, > technical debt reduction, and coding style adjustments much easier. :) Well, we do have rather a lot of linters these days. That's actually something I've been meaning to raise - perhaps we could start thinking about some pluggable way of running linters so that they're all run as part of a normal kernel build (and something that would be easy to drop new linters in to; I'd like to write some bcachefs specific ones). The current model of "I have to remember to run these 5 things, and then I'm going to get email nags for 3 more that I can't run" is not terribly scalable :)
