Hi all,
After merging the v9fs tree, today's linux-next build (i386 defconfig)
failed like this:
In file included from include/linux/kernel.h:29,
from arch/x86/include/asm/percpu.h:27,
from arch/x86/include/asm/nospec-branch.h:14,
from arch/x86/include/asm/paravirt_types.h:27,
from arch/x86/include/asm/ptrace.h:97,
from arch/x86/include/asm/math_emu.h:5,
from arch/x86/include/asm/processor.h:13,
from arch/x86/include/asm/timex.h:5,
from include/linux/timex.h:67,
from include/linux/time32.h:13,
from include/linux/time.h:60,
from include/linux/stat.h:19,
from include/linux/module.h:13,
from net/9p/client.c:11:
net/9p/client.c: In function 'p9_check_errors':
include/linux/kern_levels.h:5:25: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
include/linux/printk.h:429:25: note: in definition of macro 'printk_index_wrap'
429 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~
include/linux/printk.h:500:9: note: in expansion of macro 'printk'
500 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~
include/linux/kern_levels.h:11:25: note: in expansion of macro 'KERN_SOH'
11 | #define KERN_ERR KERN_SOH "3" /* error conditions */
| ^~~~~~~~
include/linux/printk.h:500:16: note: in expansion of macro 'KERN_ERR'
500 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~
net/9p/client.c:523:17: note: in expansion of macro 'pr_err'
523 | pr_err(
| ^~~~~~
cc1: all warnings being treated as errors
Caused by commit
36cd2f80abf8 ("net/9p: fix response size check in p9_check_errors()")
I have applied the following patch for today:
From: Stephen Rothwell <[email protected]>
Date: Mon, 5 Dec 2022 14:55:10 +1100
Subject: [PATCH] net/9p: use %zu to print size_t
Fixes: 36cd2f80abf8 ("net/9p: fix response size check in p9_check_errors()")
Signed-off-by: Stephen Rothwell <[email protected]>
---
net/9p/client.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/9p/client.c b/net/9p/client.c
index deb7baa178f3..6db5e0c55f9c 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -521,7 +521,7 @@ static int p9_check_errors(struct p9_client *c, struct p9_req_t *req)
err = p9_parse_header(&req->rc, NULL, &type, NULL, 0);
if (req->rc.size > req->rc.capacity && !req->rc.zc) {
pr_err(
- "requested packet size too big: %d does not fit %ld (type=%d)\n",
+ "requested packet size too big: %d does not fit %zu (type=%d)\n",
req->rc.size, req->rc.capacity, req->rc.id);
return -EIO;
}
--
2.35.1
--
Cheers,
Stephen Rothwell
Stephen Rothwell wrote on Mon, Dec 05, 2022 at 03:03:16PM +1100:
> After merging the v9fs tree, today's linux-next build (i386 defconfig)
> failed like this:
>
> In file included from include/linux/kernel.h:29,
> from arch/x86/include/asm/percpu.h:27,
> from arch/x86/include/asm/nospec-branch.h:14,
> from arch/x86/include/asm/paravirt_types.h:27,
> from arch/x86/include/asm/ptrace.h:97,
> from arch/x86/include/asm/math_emu.h:5,
> from arch/x86/include/asm/processor.h:13,
> from arch/x86/include/asm/timex.h:5,
> from include/linux/timex.h:67,
> from include/linux/time32.h:13,
> from include/linux/time.h:60,
> from include/linux/stat.h:19,
> from include/linux/module.h:13,
> from net/9p/client.c:11:
> net/9p/client.c: In function 'p9_check_errors':
> include/linux/kern_levels.h:5:25: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
> 5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
> | ^~~~~~
> include/linux/printk.h:429:25: note: in definition of macro 'printk_index_wrap'
> 429 | _p_func(_fmt, ##__VA_ARGS__); \
> | ^~~~
> include/linux/printk.h:500:9: note: in expansion of macro 'printk'
> 500 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
> | ^~~~~~
> include/linux/kern_levels.h:11:25: note: in expansion of macro 'KERN_SOH'
> 11 | #define KERN_ERR KERN_SOH "3" /* error conditions */
> | ^~~~~~~~
> include/linux/printk.h:500:16: note: in expansion of macro 'KERN_ERR'
> 500 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
> | ^~~~~~~~
> net/9p/client.c:523:17: note: in expansion of macro 'pr_err'
> 523 | pr_err(
> | ^~~~~~
> cc1: all warnings being treated as errors
>
> Caused by commit
>
> 36cd2f80abf8 ("net/9p: fix response size check in p9_check_errors()")
>
> I have applied the following patch for today:
Thank you!
I guess I should start building one 32bit kernel somewhere...
I've amended the bad commit with this and added a note to the patch
thanking you (not quite sure how to express that with xx-by: like tags,
it's just words -- if you care and have a suggestion feel free)
--
Dominique
Hi Dominique,
On Mon, 5 Dec 2022 13:10:18 +0900 Dominique Martinet <[email protected]> wrote:
>
> I've amended the bad commit with this and added a note to the patch
> thanking you (not quite sure how to express that with xx-by: like tags,
> it's just words -- if you care and have a suggestion feel free)
Looks good to me.
--
Cheers,
Stephen Rothwell
On Monday, December 5, 2022 5:10:18 AM CET Dominique Martinet wrote:
> Stephen Rothwell wrote on Mon, Dec 05, 2022 at 03:03:16PM +1100:
> > After merging the v9fs tree, today's linux-next build (i386 defconfig)
> > failed like this:
[...]
> > net/9p/client.c:523:17: note: in expansion of macro 'pr_err'
> > 523 | pr_err(
> > | ^~~~~~
> > cc1: all warnings being treated as errors
> >
> > Caused by commit
> >
> > 36cd2f80abf8 ("net/9p: fix response size check in p9_check_errors()")
> >
> > I have applied the following patch for today:
>
> Thank you!
> I guess I should start building one 32bit kernel somewhere...
:/ I'll setup a 32-bit build system as well, sorry!
Best regards,
Christian Schoenebeck
On Monday, December 5, 2022 3:31:57 PM CET Christian Schoenebeck wrote:
> On Monday, December 5, 2022 5:10:18 AM CET Dominique Martinet wrote:
> > Stephen Rothwell wrote on Mon, Dec 05, 2022 at 03:03:16PM +1100:
> > > After merging the v9fs tree, today's linux-next build (i386 defconfig)
>
> > > failed like this:
> [...]
>
> > > net/9p/client.c:523:17: note: in expansion of macro 'pr_err'
> > >
> > > 523 | pr_err(
> > >
> > > | ^~~~~~
> > >
> > > cc1: all warnings being treated as errors
> > >
> > > Caused by commit
> > >
> > > 36cd2f80abf8 ("net/9p: fix response size check in p9_check_errors()")
> > >
> > > I have applied the following patch for today:
> > Thank you!
> > I guess I should start building one 32bit kernel somewhere...
> :
> :/ I'll setup a 32-bit build system as well, sorry!
Dominique, looking at your 9p queue, I just realized what happened here: I
posted a v2 of these two patches, which got lost for some reason:
https://lore.kernel.org/all/[email protected]/
The currently queued 1st patch is still v1 as well.
Best regards,
Christian Schoenebeck
Christian Schoenebeck wrote on Mon, Dec 05, 2022 at 09:40:06PM +0100:
> Dominique, looking at your 9p queue, I just realized what happened here: I
> posted a v2 of these two patches, which got lost for some reason:
>
> https://lore.kernel.org/all/[email protected]/
>
> The currently queued 1st patch is still v1 as well.
Oh. Now how did I manage that one..
Thanks for the catch, and v2 had the valid printf modifier...
Sorry for all the trouble :(
--
Dominique
On Monday, December 5, 2022 11:41:55 PM CET Dominique Martinet wrote:
> Christian Schoenebeck wrote on Mon, Dec 05, 2022 at 09:40:06PM +0100:
> > Dominique, looking at your 9p queue, I just realized what happened here: I
> > posted a v2 of these two patches, which got lost for some reason:
> >
> > https://lore.kernel.org/all/[email protected]/
> >
> > The currently queued 1st patch is still v1 as well.
>
> Oh. Now how did I manage that one..
> Thanks for the catch, and v2 had the valid printf modifier...
You remember updating the 1st patch as well, right? :)
In general, I'm sure nobody complains about extra noise like "queued on...".
Then it's also more likely for other people to get which patches are still
pending or unseen.
Christian Schoenebeck wrote on Thu, Dec 08, 2022 at 04:55:17PM +0100:
> On Monday, December 5, 2022 11:41:55 PM CET Dominique Martinet wrote:
> > Christian Schoenebeck wrote on Mon, Dec 05, 2022 at 09:40:06PM +0100:
> > > Dominique, looking at your 9p queue, I just realized what happened here: I
> > > posted a v2 of these two patches, which got lost for some reason:
> > >
> > > https://lore.kernel.org/all/[email protected]/
> > >
> > > The currently queued 1st patch is still v1 as well.
> >
> > Oh. Now how did I manage that one..
> > Thanks for the catch, and v2 had the valid printf modifier...
>
> You remember updating the 1st patch as well, right? :)
It looks up to date to me, e.g. zc is added at the end of the p9_fcall
structure.
(and these are the only two patches you sent, right? :D)
> In general, I'm sure nobody complains about extra noise like "queued on...".
> Then it's also more likely for other people to get which patches are still
> pending or unseen.
I usually apply the patch locally when writing a note about 'taking the
patch for x' -- but the problem is my workflow is pretty manual to say
the least (piping mail to base64, base64 to git am on another
machine...); and I'm not always taking the time to run tests immediately
so not pushing right away to -next, so I assume I took your patches
early and looked back when testing after you sent v2 and they were there
so did't notice :/
I guess I need to pull the tree back and script a reply from the last
link or something; so you'll notice the reply is on v1 in this case?
but it'll be a pain to get the subject back like e.g. pwbot does for
netdev... hmm..
I'll think about what I can do.
--
Dominique
On Friday, December 9, 2022 12:53:35 AM CET Dominique Martinet wrote:
> Christian Schoenebeck wrote on Thu, Dec 08, 2022 at 04:55:17PM +0100:
> > On Monday, December 5, 2022 11:41:55 PM CET Dominique Martinet wrote:
> > > Christian Schoenebeck wrote on Mon, Dec 05, 2022 at 09:40:06PM +0100:
> > > > Dominique, looking at your 9p queue, I just realized what happened here: I
> > > > posted a v2 of these two patches, which got lost for some reason:
> > > >
> > > > https://lore.kernel.org/all/[email protected]/
> > > >
> > > > The currently queued 1st patch is still v1 as well.
> > >
> > > Oh. Now how did I manage that one..
> > > Thanks for the catch, and v2 had the valid printf modifier...
> >
> > You remember updating the 1st patch as well, right? :)
>
> It looks up to date to me, e.g. zc is added at the end of the p9_fcall
> structure.
> (and these are the only two patches you sent, right? :D)
Mmm, that's the queued patch I see:
https://github.com/martinetd/linux/commit/298468c26c14682a5be80a6ec1b4880c8eb3b261
Which is v1 ('zc' is not at the end of the structure, and in v1 there were
multiple assignment in the same line like:
req->tc.zc = req->rc.zc = false;
which caused code style checker to bark (as well as on the commit log which it
found too short). So in v2 it is:
req->tc.zc = false;
req->rc.zc = false;
And yes, only two patches. :)
> > In general, I'm sure nobody complains about extra noise like "queued on...".
> > Then it's also more likely for other people to get which patches are still
> > pending or unseen.
>
> I usually apply the patch locally when writing a note about 'taking the
> patch for x' -- but the problem is my workflow is pretty manual to say
> the least (piping mail to base64, base64 to git am on another
> machine...); and I'm not always taking the time to run tests immediately
> so not pushing right away to -next, so I assume I took your patches
> early and looked back when testing after you sent v2 and they were there
> so did't notice :/
>
> I guess I need to pull the tree back and script a reply from the last
> link or something; so you'll notice the reply is on v1 in this case?
> but it'll be a pain to get the subject back like e.g. pwbot does for
> netdev... hmm..
> I'll think about what I can do.
Well, workflows are quite different. Personally I always manually reply to
mailed patches once I queued them, so that people can verify and correct me in
case I queued the wrong ones. I never had the feeling to script that part.
There are also people that use tools to keep track of all incoming patches,
e.g. patchwork client (either the CLI tool, or the web interface):
https://patchwork.readthedocs.io/en/latest/usage/clients/
The advantage is that it can take care about submitted versions and you just
mark queued series as done. If you don't have an account it is read-only, so
you can view pending patches, their status, automatically apply them, but for
changing their official status somebody has to create an account for you:
https://lore.kernel.org/all/2915273.dLz0rCdnKo@silver/
There are various patchwork hosts. Not sure who is responsible for creating
patchwork accounts on lore.kernel.org.
The Redhat guys are using Patchew, e.g.:
https://patchew.org/QEMU/
Christian Schoenebeck wrote on Fri, Dec 09, 2022 at 03:40:06PM +0100:
> > > You remember updating the 1st patch as well, right? :)
> >
> > It looks up to date to me, e.g. zc is added at the end of the p9_fcall
> > structure.
> > (and these are the only two patches you sent, right? :D)
>
> Mmm, that's the queued patch I see:
> https://github.com/martinetd/linux/commit/298468c26c14682a5be80a6ec1b4880c8eb3b261
>
> Which is v1 ('zc' is not at the end of the structure, and in v1 there were
> multiple assignment in the same line like:
>
> req->tc.zc = req->rc.zc = false;
>
> which caused code style checker to bark (as well as on the commit log which it
> found too short). So in v2 it is:
>
> req->tc.zc = false;
> req->rc.zc = false;
>
> And yes, only two patches. :)
Ah. . . what did I just say about applying patches in my local branch
for testing later, they correct one is just sitting there but wasn't
tested/pushed yet :/
(if you care, I'm using my 9p-test branch for that, but it's not sent
sent to -next obviously)
> Well, workflows are quite different. Personally I always manually reply to
> mailed patches once I queued them, so that people can verify and correct me in
> case I queued the wrong ones. I never had the feeling to script that part.
Yes I usually do write a note about it when I take the patch locally,
but in this case I think I just applied the patches for checkpatch
(indentation looked off without being sure it'd complain) and didn't
intend to queue it; then came back later and "oh they're here, thanks
past me!" (incorrectly)
I guess at this point the problem comes back down to not running
tests/pushing to next immediately; if I finish automating that part I
think this kind of errors wouldn't happen as non-pushed patches wouldn't
make sense...
Well, it's been rare enough but still worth thinking about safeguards
imo, there's usually a reason for v2 patchs so getting the v1 in even
occasionally is bad :-D
--
Dominique