[PATCH v2 6/6] serial: 8250: make port operations work on struct uart_8250_port
Alejandro Colomar
alx+linux-hardening at kernel.org
Tue Oct 13 03:51:48 PDT 2026
Hi Jiri,
> Date: 2026-10-13 11:39:00+0200
> From: Jiri Slaby <jirislaby at kernel.org>
>
> On 13. 10. 26, 11:09, Andy Shevchenko wrote:
> > On Tue, Oct 13, 2026 at 09:53:26AM +0200, Jiri Slaby wrote:
> > > On 13. 10. 26, 9:33, Andy Shevchenko wrote:
> > > > On Tue, Oct 13, 2026 at 08:48:51AM +0200, Jiri Slaby (SUSE) wrote:
> >
> > ...
> >
> > > > > - rc = serial8250_register_8250_port(&port);
> > > > > - if (rc < 0)
> > > > > + struct uart_8250_port *uport = serial8250_register_8250_port(&port);
> > > > > + if (IS_ERR(uport)) {
> > > > > + rc = PTR_ERR(uport);
> > > > > goto err_sysfs_remove;
> > > > > + }
> > >
> > > > Also do not define
> > > > variables in the middle of the code (it's only allowed for auto cleanup
> > > > variables and for-loop iterators).
> > >
> > > Why, actually?
> >
> > Because it's the style.
>
> There is no hard style though. It varies across subsystems.
>
> > Linus also told it many times
>
> Which is neither an argument. Linus is wrong at times ("Link:" use as an
> example).
>
> All in all, I do not insist on this at all. It's easier to read and follow
> the code for me. Not necessarily the case for others...
>
> > > First, it is used in the kernel quite a few times already
> > > [1]. Second, while it is sort of c++ism (and I like to use modern c++), it
> > > makes the life of the variable a lot more obvious.
> > >
> > > [1] eg. see output of: rg -U -C 5 '}\n+\s+struct .*\*.*=' drivers/
> >
> > (you made me to install something I haven't heard about, it's better to use
> > `git grep` though)
>
> rg is order of magnitude faster (searches in parallel). More importantly in
> this case, supports multiline matches. (And respects .git ignores.)
Is that really true?
alx at debian:~/src/linux/linux/master$ time \
find * -type f \
| grep '\.[ch]' \
| sort \
| xargs grep -n ARRAY_SIZE /dev/null \
| wc -l;
58852
real 0m1.022s
user 0m0.726s
sys 0m0.358s
alx at debian:~/src/linux/linux/master$ time \
git ls-files \
| grep '\.[ch]' \
| xargs grep -n ARRAY_SIZE /dev/null \
| wc -l;
58854
real 0m0.923s
user 0m0.650s
sys 0m0.333s
There seems to be a difference of two occurences, which is due to
symlinks:
alx at debian:~/src/linux/linux/master$ diff -u \
<(find * -type f \
| grep '\.[ch]' \
| sort \
| xargs grep -n ARRAY_SIZE /dev/null) \
<(git ls-files \
| grep '\.[ch]' \
| xargs grep -n ARRAY_SIZE /dev/null);
--- /dev/fd/63 2026-10-13 12:40:11.675302047 +0200
+++ /dev/fd/62 2026-10-13 12:40:11.675302047 +0200
@@ -57769,6 +57769,7 @@
tools/testing/selftests/bpf/benchs/bench_sockmap.c:128: for (i = 0; i < ARRAY_SIZE(ctx.fds); i++) {
tools/testing/selftests/bpf/bpf_util.h:69:#ifndef ARRAY_SIZE
tools/testing/selftests/bpf/bpf_util.h:70:# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+tools/testing/selftests/bpf/disasm.c:23: BUILD_BUG_ON(ARRAY_SIZE(func_id_str) != __BPF_FUNC_MAX_ID);
tools/testing/selftests/bpf/map_tests/lpm_trie_map_basic_ops.c:1006: const char *sorted_keys[ARRAY_SIZE(keys)];
tools/testing/selftests/bpf/map_tests/lpm_trie_map_basic_ops.c:1012: fd = lpm_trie_create(sizeof(key), sizeof(value), ARRAY_SIZE(keys));
tools/testing/selftests/bpf/map_tests/lpm_trie_map_basic_ops.c:1014: for (i = 0; i < ARRAY_SIZE(keys); i++) {
@@ -58786,6 +58787,7 @@
tools/testing/selftests/vfio/lib/drivers/dsa/dsa.c:215: dsa->max_copies_per_batch = min(dsa->max_copies_per_batch, ARRAY_SIZE(dsa->copy));
tools/testing/selftests/vfio/lib/drivers/dsa/dsa.c:340: for (i = 0; i < ARRAY_SIZE(dsa->copy); i++) {
tools/testing/selftests/vfio/lib/drivers/dsa/dsa.c:349: for (i = 0; i < ARRAY_SIZE(dsa->batch) && count; i++) {
+tools/testing/selftests/vfio/lib/drivers/igb/e1000_82575.h:240:#define E1000_VLVF_ARRAY_SIZE 32
tools/testing/selftests/vfio/lib/drivers/nv_falcon/hw.h:249:#define VERIFIED_GPU_MAP_SIZE ARRAY_SIZE(verified_gpu_map)
tools/testing/selftests/vfio/lib/iommu.c:59: for (i = 0; i < ARRAY_SIZE(iommu_modes); i++) {
tools/testing/selftests/vfio/lib/sysfs.c:48: VFIO_ASSERT_GT(read(fd, buf, ARRAY_SIZE(buf)), 0);
And here goes rg(1):
alx at debian:~/src/linux/linux/master$ time rg -n ARRAY_SIZE | wc -l
58957
real 0m0.512s
user 0m0.490s
sys 0m0.974s
It's around 50% faster, which I wouldn't call orders of magnitude.
rg(1) inherits the bogus design of find(1) in trying to filter the file
names with multiple subtly different options. grep(1) can do that much
better, and by using a pipe, you have a dedicated process (often in a
dedicated CPU) for that task, which is likely more efficient.
And here's plain grep(1):
alx at debian:~/src/linux/linux/master$ time grep -r -n ARRAY_SIZE \
| wc -l
58957
real 0m1.140s
user 0m0.663s
sys 0m0.489s
Multiline matches are also supported by pcre2grep(1), BTW, which uses
PCRE2, which are way more useful than that.
pcre2grep(1) is itself a bit slower than grep(1) --especially if you try
very complex regexes--:
alx at debian:~/src/linux/linux/master$ time \
find * -type f \
| grep '\.[ch]' \
| sort \
| xargs pcre2grep -n ARRAY_SIZE /dev/null \
| wc -l;
58852
real 0m1.857s
user 0m1.408s
sys 0m0.516s
But if you pre-filter the files with a simpler regex, then it comes back
to usual time:
alx at debian:~/src/linux/linux/master$ time \
find * -type f \
| grep '\.[ch]' \
| sort \
| xargs grep -l ARRAY_SIZE \
| xargs pcre2grep -n ARRAY_SIZE /dev/null \
| wc -l;
58852
real 0m0.919s
user 0m0.830s
sys 0m0.462s
Have a lovely day!
Alex
> > Note excluded __free() if I get regexp correctly
> >
> > $ rg -U -l '}\n+\s+struct .*\*[^)]*=' drivers/ | less
> > drivers/ufs/host/ufs-qcom.c
> ...
> > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> >
> > So, less than dozen of drivers doing wrong is not an argument, try something
> > better. But thanks, I will check and address this in some of the above.
>
> Well, it's only an example of a declaration after a previous opening brace
> "{" -- this one turned out to be easy to search for. There is a lot of more,
> of course. kernel/ included.
>
> thanks,
> --
> js
> suse labs
>
--
<https://www.alejandro-colomar.es>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20261013/1aa95bcb/attachment-0001.sig>
More information about the linux-arm-kernel
mailing list