Message ID | 20240919195454.73358-1-kerneljasonxing@gmail.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | BPF |
Headers | show |
Series | [bpf] bpf: syscall_nrs: fix no previous prototype for "syscall_defines" | expand |
On Thu, Sep 19, 2024 at 9:55 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > From: Jason Xing <kernelxing@tencent.com> > > In some environments (gcc treated as error in W=1, which is default), if we > make -C samples/bpf/, it will be stopped because of > "no previous prototype" error like this: > > ../samples/bpf/syscall_nrs.c:7:6: > error: no previous prototype for ‘syscall_defines’ [-Werror=missing-prototypes] > void syscall_defines(void) > ^~~~~~~~~~~~~~~ samples/bpf/ doesn't accept patches any more. If this samples/test is useful, refactor it to the test_progs framework. Otherwise delete it. pw-bot: cr
On Thu, Sep 19, 2024 at 11:17 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Thu, Sep 19, 2024 at 9:55 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > From: Jason Xing <kernelxing@tencent.com> > > > > In some environments (gcc treated as error in W=1, which is default), if we > > make -C samples/bpf/, it will be stopped because of > > "no previous prototype" error like this: > > > > ../samples/bpf/syscall_nrs.c:7:6: > > error: no previous prototype for ‘syscall_defines’ [-Werror=missing-prototypes] > > void syscall_defines(void) > > ^~~~~~~~~~~~~~~ > > samples/bpf/ doesn't accept patches any more. Thanks for your reminder. I didn't know that. > If this samples/test is useful, refactor it to the test_progs framework. > Otherwise delete it. I'm trying to use the samples/test to do some tests to verify if some functions like bpf_sock_ops_cb_flags_set() can work. Then I encountered the issue as this patch said.
On Thu, Sep 19, 2024 at 11:17 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Thu, Sep 19, 2024 at 9:55 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > From: Jason Xing <kernelxing@tencent.com> > > > > In some environments (gcc treated as error in W=1, which is default), if we > > make -C samples/bpf/, it will be stopped because of > > "no previous prototype" error like this: > > > > ../samples/bpf/syscall_nrs.c:7:6: > > error: no previous prototype for ‘syscall_defines’ [-Werror=missing-prototypes] > > void syscall_defines(void) > > ^~~~~~~~~~~~~~~ > > samples/bpf/ doesn't accept patches any more. > If this samples/test is useful, refactor it to the test_progs framework. > Otherwise delete it. > > pw-bot: cr After reconsidering what Alexei said, I still feel we could take this patch? It is because: 1) the patch itself is more of a fix instead of optimization, 2)as long as samples/bpf exists in the kernel, we cannot easily let it(issues) go and ignore it. Applying such a patch won't cause any further confusion, right? As we can see, it's like a fix which does not introduce anything new here. What do you bpf maintainers think? Thanks, Jason
On Fri, Sep 20, 2024 at 12:37 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > On Thu, Sep 19, 2024 at 11:17 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Thu, Sep 19, 2024 at 9:55 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > > > From: Jason Xing <kernelxing@tencent.com> > > > > > > In some environments (gcc treated as error in W=1, which is default), if we > > > make -C samples/bpf/, it will be stopped because of > > > "no previous prototype" error like this: > > > > > > ../samples/bpf/syscall_nrs.c:7:6: > > > error: no previous prototype for ‘syscall_defines’ [-Werror=missing-prototypes] > > > void syscall_defines(void) > > > ^~~~~~~~~~~~~~~ > > > > samples/bpf/ doesn't accept patches any more. > > If this samples/test is useful, refactor it to the test_progs framework. > > Otherwise delete it. > > > > pw-bot: cr > > After reconsidering what Alexei said, I still feel we could take this > patch? It is because: > 1) the patch itself is more of a fix instead of optimization, > 2)as long as samples/bpf exists in the kernel, we cannot easily let > it(issues) go and ignore it. > > Applying such a patch won't cause any further confusion, right? As we > can see, it's like a fix which does not introduce anything new here. > > What do you bpf maintainers think? I think it's fine to minimally fix the issue in samples/bpf, but I don't think this weirdly-looking extra declaration is the best fix. Can you mark that function static? Will that work? Or, as a plan B, use pragma to disable this warning, it's clearly "expected" in this case. > > Thanks, > Jason
Hello Andrii, On Sat, Sep 28, 2024 at 7:08 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Fri, Sep 20, 2024 at 12:37 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > On Thu, Sep 19, 2024 at 11:17 PM Alexei Starovoitov > > <alexei.starovoitov@gmail.com> wrote: > > > > > > On Thu, Sep 19, 2024 at 9:55 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > > > > > From: Jason Xing <kernelxing@tencent.com> > > > > > > > > In some environments (gcc treated as error in W=1, which is default), if we > > > > make -C samples/bpf/, it will be stopped because of > > > > "no previous prototype" error like this: > > > > > > > > ../samples/bpf/syscall_nrs.c:7:6: > > > > error: no previous prototype for ‘syscall_defines’ [-Werror=missing-prototypes] > > > > void syscall_defines(void) > > > > ^~~~~~~~~~~~~~~ > > > > > > samples/bpf/ doesn't accept patches any more. > > > If this samples/test is useful, refactor it to the test_progs framework. > > > Otherwise delete it. > > > > > > pw-bot: cr > > > > After reconsidering what Alexei said, I still feel we could take this > > patch? It is because: > > 1) the patch itself is more of a fix instead of optimization, > > 2)as long as samples/bpf exists in the kernel, we cannot easily let > > it(issues) go and ignore it. > > > > Applying such a patch won't cause any further confusion, right? As we > > can see, it's like a fix which does not introduce anything new here. > > > > What do you bpf maintainers think? > > I think it's fine to minimally fix the issue in samples/bpf, but I > don't think this weirdly-looking extra declaration is the best fix. Thanks for your reply. > > Can you mark that function static? Will that work? Not really, it will print: samples/bpf/syscall_nrs.c:7:13: error: ‘syscall_defines’ defined but not used [-Werror=unused-function] static void syscall_defines(void) ^~~~~~~~~~~~~~~ cc1: all warnings being treated as errors > Or, as a plan B, > use pragma to disable this warning, it's clearly "expected" in this > case. Yes, I admit the use in this function is "expected" like you said because this file will be converted into a .h file. Could you kindly show me more hints on how to disable the warning when compiling? I tried to remove something like "-Wmissing-prototypes", but the warning still happens. Thanks, Jason
On Fri, Sep 27, 2024 at 10:22 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > Hello Andrii, > > On Sat, Sep 28, 2024 at 7:08 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Fri, Sep 20, 2024 at 12:37 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > > > On Thu, Sep 19, 2024 at 11:17 PM Alexei Starovoitov > > > <alexei.starovoitov@gmail.com> wrote: > > > > > > > > On Thu, Sep 19, 2024 at 9:55 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > > > > > > > From: Jason Xing <kernelxing@tencent.com> > > > > > > > > > > In some environments (gcc treated as error in W=1, which is default), if we > > > > > make -C samples/bpf/, it will be stopped because of > > > > > "no previous prototype" error like this: > > > > > > > > > > ../samples/bpf/syscall_nrs.c:7:6: > > > > > error: no previous prototype for ‘syscall_defines’ [-Werror=missing-prototypes] > > > > > void syscall_defines(void) > > > > > ^~~~~~~~~~~~~~~ > > > > > > > > samples/bpf/ doesn't accept patches any more. > > > > If this samples/test is useful, refactor it to the test_progs framework. > > > > Otherwise delete it. > > > > > > > > pw-bot: cr > > > > > > After reconsidering what Alexei said, I still feel we could take this > > > patch? It is because: > > > 1) the patch itself is more of a fix instead of optimization, > > > 2)as long as samples/bpf exists in the kernel, we cannot easily let > > > it(issues) go and ignore it. > > > > > > Applying such a patch won't cause any further confusion, right? As we > > > can see, it's like a fix which does not introduce anything new here. > > > > > > What do you bpf maintainers think? > > > > I think it's fine to minimally fix the issue in samples/bpf, but I > > don't think this weirdly-looking extra declaration is the best fix. > > Thanks for your reply. > > > > > Can you mark that function static? Will that work? > > Not really, it will print: > samples/bpf/syscall_nrs.c:7:13: error: ‘syscall_defines’ defined but > not used [-Werror=unused-function] > static void syscall_defines(void) > ^~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors > > > Or, as a plan B, > > use pragma to disable this warning, it's clearly "expected" in this > > case. > > Yes, I admit the use in this function is "expected" like you said > because this file will be converted into a .h file. Could you kindly > show me more hints on how to disable the warning when compiling? I > tried to remove something like "-Wmissing-prototypes", but the warning > still happens. > Grep for "#pragma GCC diagnostic ignored" uses in kernel sources. > Thanks, > Jason
On Tue, Oct 1, 2024 at 6:06 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Fri, Sep 27, 2024 at 10:22 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > Hello Andrii, > > > > On Sat, Sep 28, 2024 at 7:08 AM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Fri, Sep 20, 2024 at 12:37 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > > > > > On Thu, Sep 19, 2024 at 11:17 PM Alexei Starovoitov > > > > <alexei.starovoitov@gmail.com> wrote: > > > > > > > > > > On Thu, Sep 19, 2024 at 9:55 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > > > > > > > > > From: Jason Xing <kernelxing@tencent.com> > > > > > > > > > > > > In some environments (gcc treated as error in W=1, which is default), if we > > > > > > make -C samples/bpf/, it will be stopped because of > > > > > > "no previous prototype" error like this: > > > > > > > > > > > > ../samples/bpf/syscall_nrs.c:7:6: > > > > > > error: no previous prototype for ‘syscall_defines’ [-Werror=missing-prototypes] > > > > > > void syscall_defines(void) > > > > > > ^~~~~~~~~~~~~~~ > > > > > > > > > > samples/bpf/ doesn't accept patches any more. > > > > > If this samples/test is useful, refactor it to the test_progs framework. > > > > > Otherwise delete it. > > > > > > > > > > pw-bot: cr > > > > > > > > After reconsidering what Alexei said, I still feel we could take this > > > > patch? It is because: > > > > 1) the patch itself is more of a fix instead of optimization, > > > > 2)as long as samples/bpf exists in the kernel, we cannot easily let > > > > it(issues) go and ignore it. > > > > > > > > Applying such a patch won't cause any further confusion, right? As we > > > > can see, it's like a fix which does not introduce anything new here. > > > > > > > > What do you bpf maintainers think? > > > > > > I think it's fine to minimally fix the issue in samples/bpf, but I > > > don't think this weirdly-looking extra declaration is the best fix. > > > > Thanks for your reply. > > > > > > > > Can you mark that function static? Will that work? > > > > Not really, it will print: > > samples/bpf/syscall_nrs.c:7:13: error: ‘syscall_defines’ defined but > > not used [-Werror=unused-function] > > static void syscall_defines(void) > > ^~~~~~~~~~~~~~~ > > cc1: all warnings being treated as errors > > > > > Or, as a plan B, > > > use pragma to disable this warning, it's clearly "expected" in this > > > case. > > > > Yes, I admit the use in this function is "expected" like you said > > because this file will be converted into a .h file. Could you kindly > > show me more hints on how to disable the warning when compiling? I > > tried to remove something like "-Wmissing-prototypes", but the warning > > still happens. > > > > Grep for "#pragma GCC diagnostic ignored" uses in kernel sources. Thanks a lot! It works. Let me re-post it to "fix" this issue. Thanks, Jason
diff --git a/samples/bpf/syscall_nrs.c b/samples/bpf/syscall_nrs.c index 88f940052450..5f40a29034b6 100644 --- a/samples/bpf/syscall_nrs.c +++ b/samples/bpf/syscall_nrs.c @@ -4,6 +4,8 @@ #define SYSNR(_NR) DEFINE(SYS ## _NR, _NR) +void syscall_defines(void); + void syscall_defines(void) { COMMENT("Linux system call numbers.");