| Message ID | 20241029054651.86356-2-mtodorovac69@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] fs/proc/kcore.c: fix coccinelle reported ERROR instances | expand |
On Tue, 29 Oct 2024 06:46:52 +0100 Mirsad Todorovac <mtodorovac69@gmail.com> wrote: > Coccinelle complains about the nested reuse of the pointer `iter' with different > pointer type: > > ./fs/proc/kcore.c:515:26-30: ERROR: invalid reference to the index variable of the iterator on line 499 > ./fs/proc/kcore.c:534:23-27: ERROR: invalid reference to the index variable of the iterator on line 499 > ./fs/proc/kcore.c:550:40-44: ERROR: invalid reference to the index variable of the iterator on line 499 > ./fs/proc/kcore.c:568:27-31: ERROR: invalid reference to the index variable of the iterator on line 499 > ./fs/proc/kcore.c:581:28-32: ERROR: invalid reference to the index variable of the iterator on line 499 > ./fs/proc/kcore.c:599:27-31: ERROR: invalid reference to the index variable of the iterator on line 499 > ./fs/proc/kcore.c:607:38-42: ERROR: invalid reference to the index variable of the iterator on line 499 > ./fs/proc/kcore.c:614:26-30: ERROR: invalid reference to the index variable of the iterator on line 499 > > Replacing `struct kcore_list *iter' with `struct kcore_list *tmp' doesn't change the > scope and the functionality is the same and coccinelle seems happy. Well that's dumb of it. Still, the code is presently a bit weird and we don't mind working around such third-party issues. > NOTE: There was an issue with using `struct kcore_list *pos' as the nested iterator. > The build did not work! It worked for me. What's wrong with that? > --- a/fs/proc/kcore.c > +++ b/fs/proc/kcore.c > @@ -493,13 +493,13 @@ static ssize_t read_kcore_iter(struct kiocb *iocb, struct iov_iter *iter) > * the previous entry, search for a matching entry. > */ > if (!m || start < m->addr || start >= m->addr + m->size) { > - struct kcore_list *iter; > + struct kcore_list *tmp; `tmp' is a really poor identifier :( Let's try `pos': --- a/fs/proc/kcore.c~fs-proc-kcorec-fix-coccinelle-reported-error-instances-fix +++ a/fs/proc/kcore.c @@ -493,13 +493,13 @@ static ssize_t read_kcore_iter(struct ki * the previous entry, search for a matching entry. */ if (!m || start < m->addr || start >= m->addr + m->size) { - struct kcore_list *tmp; + struct kcore_list *pos; m = NULL; - list_for_each_entry(tmp, &kclist_head, list) { - if (start >= tmp->addr && - start < tmp->addr + tmp->size) { - m = tmp; + list_for_each_entry(pos, &kclist_head, list) { + if (start >= pos->addr && + start < pos->addr + pos->size) { + m = pos; break; } }
Hi, Mr. Andrew, On 10/30/24 02:29, Andrew Morton wrote: > On Tue, 29 Oct 2024 06:46:52 +0100 Mirsad Todorovac <mtodorovac69@gmail.com> wrote: > >> Coccinelle complains about the nested reuse of the pointer `iter' with different >> pointer type: >> >> ./fs/proc/kcore.c:515:26-30: ERROR: invalid reference to the index variable of the iterator on line 499 >> ./fs/proc/kcore.c:534:23-27: ERROR: invalid reference to the index variable of the iterator on line 499 >> ./fs/proc/kcore.c:550:40-44: ERROR: invalid reference to the index variable of the iterator on line 499 >> ./fs/proc/kcore.c:568:27-31: ERROR: invalid reference to the index variable of the iterator on line 499 >> ./fs/proc/kcore.c:581:28-32: ERROR: invalid reference to the index variable of the iterator on line 499 >> ./fs/proc/kcore.c:599:27-31: ERROR: invalid reference to the index variable of the iterator on line 499 >> ./fs/proc/kcore.c:607:38-42: ERROR: invalid reference to the index variable of the iterator on line 499 >> ./fs/proc/kcore.c:614:26-30: ERROR: invalid reference to the index variable of the iterator on line 499 >> >> Replacing `struct kcore_list *iter' with `struct kcore_list *tmp' doesn't change the >> scope and the functionality is the same and coccinelle seems happy. > > Well that's dumb of it. Still, the code is presently a bit weird and > we don't mind working around such third-party issues. > >> NOTE: There was an issue with using `struct kcore_list *pos' as the nested iterator. >> The build did not work! > > It worked for me. What's wrong with that? Now with next-20241031 it works for me too: marvin@defiant:~/linux/kernel/linux-next$ time nice sudo make TARGETS=proc kselftest |& tee ../kself-proc-01a.log; date make[3]: Entering directory '.../linux-next/tools/testing/selftests/proc' make[3]: Nothing to be done for 'all'. make[3]: Leaving directory '.../linux-next/tools/testing/selftests/proc' make[3]: Entering directory '.../linux-next/tools/testing/selftests/proc' TAP version 13 1..23 # timeout set to 45 # selftests: proc: fd-001-lookup ok 1 selftests: proc: fd-001-lookup # timeout set to 45 # selftests: proc: fd-002-posix-eq ok 2 selftests: proc: fd-002-posix-eq # timeout set to 45 # selftests: proc: fd-003-kthread ok 3 selftests: proc: fd-003-kthread # timeout set to 45 # selftests: proc: proc-2-is-kthread ok 4 selftests: proc: proc-2-is-kthread # timeout set to 45 # selftests: proc: proc-loadavg-001 ok 5 selftests: proc: proc-loadavg-001 # timeout set to 45 # selftests: proc: proc-empty-vm ok 6 selftests: proc: proc-empty-vm # timeout set to 45 # selftests: proc: proc-pid-vm ok 7 selftests: proc: proc-pid-vm # timeout set to 45 # selftests: proc: proc-self-map-files-001 ok 8 selftests: proc: proc-self-map-files-001 # timeout set to 45 # selftests: proc: proc-self-map-files-002 ok 9 selftests: proc: proc-self-map-files-002 # timeout set to 45 # selftests: proc: proc-self-isnt-kthread ok 10 selftests: proc: proc-self-isnt-kthread # timeout set to 45 # selftests: proc: proc-self-syscall ok 11 selftests: proc: proc-self-syscall # timeout set to 45 # selftests: proc: proc-self-wchan ok 12 selftests: proc: proc-self-wchan # timeout set to 45 # selftests: proc: proc-subset-pid ok 13 selftests: proc: proc-subset-pid # timeout set to 45 # selftests: proc: proc-tid0 ok 14 selftests: proc: proc-tid0 # timeout set to 45 # selftests: proc: proc-uptime-001 ok 15 selftests: proc: proc-uptime-001 # timeout set to 45 # selftests: proc: proc-uptime-002 ok 16 selftests: proc: proc-uptime-002 # timeout set to 45 # selftests: proc: read ok 17 selftests: proc: read # timeout set to 45 # selftests: proc: self ok 18 selftests: proc: self # timeout set to 45 # selftests: proc: setns-dcache ok 19 selftests: proc: setns-dcache # timeout set to 45 # selftests: proc: setns-sysvipc ok 20 selftests: proc: setns-sysvipc # timeout set to 45 # selftests: proc: thread-self ok 21 selftests: proc: thread-self # timeout set to 45 # selftests: proc: proc-multiple-procfs ok 22 selftests: proc: proc-multiple-procfs # timeout set to 45 # selftests: proc: proc-fsconfig-hidepid ok 23 selftests: proc: proc-fsconfig-hidepid make[3]: Leaving directory '.../linux-next/tools/testing/selftests/proc' Unless I badly missed something, the build is OK. >> --- a/fs/proc/kcore.c >> +++ b/fs/proc/kcore.c >> @@ -493,13 +493,13 @@ static ssize_t read_kcore_iter(struct kiocb *iocb, struct iov_iter *iter) >> * the previous entry, search for a matching entry. >> */ >> if (!m || start < m->addr || start >= m->addr + m->size) { >> - struct kcore_list *iter; >> + struct kcore_list *tmp; > > `tmp' is a really poor identifier :( > > Let's try `pos': > > --- a/fs/proc/kcore.c~fs-proc-kcorec-fix-coccinelle-reported-error-instances-fix > +++ a/fs/proc/kcore.c > @@ -493,13 +493,13 @@ static ssize_t read_kcore_iter(struct ki > * the previous entry, search for a matching entry. > */ > if (!m || start < m->addr || start >= m->addr + m->size) { > - struct kcore_list *tmp; > + struct kcore_list *pos; > > m = NULL; > - list_for_each_entry(tmp, &kclist_head, list) { > - if (start >= tmp->addr && > - start < tmp->addr + tmp->size) { > - m = tmp; > + list_for_each_entry(pos, &kclist_head, list) { > + if (start >= pos->addr && > + start < pos->addr + pos->size) { > + m = pos; > break; > } > } I see that it is already applied in next-20241031 and it is just running. $ uname -rms Linux 6.12.0-rc5-next-20241031nxt x86_64 Please add Tested-by: Mirsad Todorovac <mtodorovac69@gmail.com> Thanks. Best regards, Mirsad
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c index 51446c59388f..25586ec4096a 100644 --- a/fs/proc/kcore.c +++ b/fs/proc/kcore.c @@ -493,13 +493,13 @@ static ssize_t read_kcore_iter(struct kiocb *iocb, struct iov_iter *iter) * the previous entry, search for a matching entry. */ if (!m || start < m->addr || start >= m->addr + m->size) { - struct kcore_list *iter; + struct kcore_list *tmp; m = NULL; - list_for_each_entry(iter, &kclist_head, list) { - if (start >= iter->addr && - start < iter->addr + iter->size) { - m = iter; + list_for_each_entry(tmp, &kclist_head, list) { + if (start >= tmp->addr && + start < tmp->addr + tmp->size) { + m = tmp; break; } }
Coccinelle complains about the nested reuse of the pointer `iter' with different pointer type: ./fs/proc/kcore.c:515:26-30: ERROR: invalid reference to the index variable of the iterator on line 499 ./fs/proc/kcore.c:534:23-27: ERROR: invalid reference to the index variable of the iterator on line 499 ./fs/proc/kcore.c:550:40-44: ERROR: invalid reference to the index variable of the iterator on line 499 ./fs/proc/kcore.c:568:27-31: ERROR: invalid reference to the index variable of the iterator on line 499 ./fs/proc/kcore.c:581:28-32: ERROR: invalid reference to the index variable of the iterator on line 499 ./fs/proc/kcore.c:599:27-31: ERROR: invalid reference to the index variable of the iterator on line 499 ./fs/proc/kcore.c:607:38-42: ERROR: invalid reference to the index variable of the iterator on line 499 ./fs/proc/kcore.c:614:26-30: ERROR: invalid reference to the index variable of the iterator on line 499 Replacing `struct kcore_list *iter' with `struct kcore_list *tmp' doesn't change the scope and the functionality is the same and coccinelle seems happy. NOTE: There was an issue with using `struct kcore_list *pos' as the nested iterator. The build did not work! Fixes: 04d168c6d42d1 ("fs/proc/kcore.c: remove check of list iterator against head past the loop body") Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1] Link: https://lkml.kernel.org/r/20220331223700.902556-1-jakobkoschel@gmail.com Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com> Cc: Mike Rapoport <rppt@kernel.org> Cc: David Hildenbrand <david@redhat.com> Cc: Oscar Salvador <osalvador@suse.de> Cc: "Brian Johannesmeyer" <bjohannesmeyer@gmail.com> Cc: Cristiano Giuffrida <c.giuffrida@vu.nl> Cc: "Bos, H.J." <h.j.bos@vu.nl> Cc: Alexey Dobriyan <adobriyan@gmail.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Yang Li <yang.lee@linux.alibaba.com> Cc: Baoquan He <bhe@redhat.com> Cc: Hari Bathini <hbathini@linux.ibm.com> Cc: Yan Zhen <yanzhen@vivo.com> Cc: Alexander Gordeev <agordeev@linux.ibm.com> Cc: linux-fsdevel@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Mirsad Todorovac <mtodorovac69@gmail.com> --- v1: initial version fs/proc/kcore.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)