From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
To: Alejandro Colomar <colomar.6.4.3@gmail.com>
Cc: mtk.manpages@gmail.com, linux-man@vger.kernel.org
Subject: Re: [patch] memusage.1, bind.2, eventfd.2, futex.2, open_by_handle_at.2, perf_event_open.2, poll.2, signalfd.2, sysctl.2, timerfd_create.2, bsearch.3, cmsg.3, getaddrinfo.3, getaddrinfo_a.3 getgrouplist.3, insque.3, malloc_info.3, mbsinit.3, mbstowcs.3, pthread_create.3, pthread_setaffinity_np.3, queue.3, rtnetlink.3, shm_open.3, strptime.3, tsearch.3, aio.7, fanotify.7, inotify.7, unix.7: Use sizeof consistently
Date: Tue, 25 Aug 2020 12:29:18 +0200 [thread overview]
Message-ID: <e1b424c2-77c1-e995-a866-67a122d7bb07@gmail.com> (raw)
In-Reply-To: <cc176298-50e2-7831-f2f7-21be8cae9075@gmail.com>
Hello Alex,
On 8/24/20 3:29 PM, Alejandro Colomar wrote:
>>From 5df5cae0fb6973df0ab8b3629934f808487112b0 Mon Sep 17 00:00:00 2001
> From: Alejandro Colomar <colomar.6.4.3@gmail.com>
> Date: Fri, 21 Aug 2020 16:58:12 +0200
> Subject: [PATCH] memusage.1, bind.2, eventfd.2, futex.2,
> open_by_handle_at.2,
> perf_event_open.2, poll.2, signalfd.2, sysctl.2, timerfd_create.2,
> bsearch.3,
> cmsg.3, getaddrinfo.3, getaddrinfo_a.3 getgrouplist.3, insque.3,
> malloc_info.3, mbsinit.3, mbstowcs.3, pthread_create.3,
> pthread_setaffinity_np.3, queue.3, rtnetlink.3, shm_open.3, strptime.3,
> tsearch.3, aio.7, fanotify.7, inotify.7, unix.7: Use sizeof consistently
>
> Use ``sizeof`` consistently through all the examples, in the following
> way:
I would really have preferred three patches here, since:
> - Never use a space after ``sizeof``, and always use parentheses
> instead.
>
> Rationale:
>
> https://www.kernel.org/doc/html/v5.8/process/coding-style.html#spaces
(1) This is completely unproblematic from my point of view.
> - Use the name of the variable instead of the type as argument
> for ``sizeof``, wherever possible.
>
> Rationale:
>
> https://www.kernel.org/doc/html/v5.8/process/coding-style.html#allocating-memory
(2) This one is up for debate. In many cases it makes sense to do
this. However, there are cases where I think that using the struct
name can actually help readability. And when I grep through the kernel
source, of around 139k lines that use "sizeof", some 37k take a 'struct type'
as an argument. SI, I think this kind of change may need to be considered on
a case by case basis, rather than as a blanket change.
> - When the result of ``sizeof`` is multiplied (or otherwise modified),
> write ``sizeof`` in the first place.
>
> Rationale:
>
> ``(sizeof(x) * INT_MAX * 2)`` doesn't overflow.
>
> ``(INT_MAX * 2 * sizeof(x))`` overflows, giving incorrect
> results.
(3) Is this true? "gcc -Wall" does not complain about this. And, I
thought that in both cases, all operands in the expression
would be promoted to the largest type. And, on my x86-64 system,
sizeof((sizeof(x) * INT_MAX * 2)) == 8
sizeof(INT_MAX * 2 * sizeof(x)) == 8
But, I will say tht I'm not a language lawyer, and C still
sometimes has surprises for me. At the least, I'd like to know
more about this point.
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
next prev parent reply other threads:[~2020-08-25 10:29 UTC|newest]
Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-24 13:29 [patch] memusage.1, bind.2, eventfd.2, futex.2, open_by_handle_at.2, perf_event_open.2, poll.2, signalfd.2, sysctl.2, timerfd_create.2, bsearch.3, cmsg.3, getaddrinfo.3, getaddrinfo_a.3 getgrouplist.3, insque.3, malloc_info.3, mbsinit.3, mbstowcs.3, pthread_create.3, pthread_setaffinity_np.3, queue.3, rtnetlink.3, shm_open.3, strptime.3, tsearch.3, aio.7, fanotify.7, inotify.7, unix.7: Use sizeof consistently Alejandro Colomar
2020-08-25 10:29 ` Michael Kerrisk (man-pages) [this message]
2020-08-25 11:19 ` Jakub Wilk
2020-08-25 11:34 ` Alejandro Colomar
2020-08-25 11:41 ` Michael Kerrisk (man-pages)
2020-08-25 11:48 ` Alejandro Colomar
2020-08-25 12:21 ` Alejandro Colomar
2020-08-25 12:35 ` Michael Kerrisk (man-pages)
2020-08-25 13:05 ` [PATCH] cmsg.3, getaddrinfo_a.3 getgrouplist.3: Use sizeof, consistently Alejandro Colomar
2020-08-26 6:21 ` Michael Kerrisk (man-pages)
2020-09-03 10:23 ` [PATCH] memusage.1: Use sizeof consistently Alejandro Colomar
2020-09-04 8:20 ` Michael Kerrisk (man-pages)
2020-09-04 10:19 ` [PATCH (2) 02/34] bind.2: " Alejandro Colomar
2020-09-04 10:21 ` [PATCH (2) 03/34] eventfd.2: " Alejandro Colomar
2020-09-04 10:46 ` Michael Kerrisk (man-pages)
2020-09-04 10:54 ` [PATCH (2) 04/34] futex.2: " Alejandro Colomar
2020-09-04 10:55 ` [PATCH (2) 05/34] open_by_handle_at.2: " Alejandro Colomar
2020-09-04 10:56 ` [PATCH (2) 06/34] perf_event_open.2: " Alejandro Colomar
2020-09-04 10:57 ` [PATCH (2) 07/34] " Alejandro Colomar
2020-09-04 12:25 ` Michael Kerrisk (man-pages)
2020-09-04 13:42 ` [PATCH (2) 08/34] poll.2: " Alejandro Colomar
2020-09-04 13:43 ` [PATCH (2) 09/34] sysctl.2: " Alejandro Colomar
2020-09-04 13:44 ` [PATCH (2) 10/34] signalfd.2: " Alejandro Colomar
2020-09-04 13:45 ` [PATCH (2) 11/34] timerfd_create.2: " Alejandro Colomar
2020-09-04 13:46 ` [PATCH (2) 12/34] bsearch.3: " Alejandro Colomar
2020-09-04 13:50 ` [PATCH (2) 13/34] cmsg.3: " Alejandro Colomar
2020-09-04 13:52 ` [PATCH (2) 14/34] " Alejandro Colomar
2020-09-04 13:53 ` [PATCH (2) 15/34] getaddrinfo.3: " Alejandro Colomar
2020-09-04 14:32 ` [PATCH (2) 16/34] " Alejandro Colomar
2020-09-04 14:34 ` [PATCH (2) 17/34] getgrouplist.3: " Alejandro Colomar
2020-09-04 14:37 ` [PATCH (2) 18/34] insque.3: " Alejandro Colomar
2020-09-04 14:41 ` [PATCH (2) 19/34] malloc_info.3: " Alejandro Colomar
2020-09-04 14:44 ` [PATCH (2) 20/34] mbsinit.3: " Alejandro Colomar
2020-09-04 14:45 ` [PATCH (2) 21/34] mbstowcs.3: " Alejandro Colomar
2020-09-04 14:48 ` [PATCH (2) 22/34] pthread_create.3: " Alejandro Colomar
2020-09-04 14:50 ` [PATCH (2) 23/34] pthread_setaffinity_np.3: " Alejandro Colomar
2020-09-04 14:50 ` [PATCH (2) 24/34] queue.3: " Alejandro Colomar
2020-09-04 14:52 ` [PATCH (2) 25/34] rtnetlink.3: " Alejandro Colomar
2020-09-04 14:54 ` [PATCH (2) 26/34] " Alejandro Colomar
2020-09-04 14:55 ` [PATCH (2) 27/34] shm_open.3: " Alejandro Colomar
2020-09-04 14:57 ` [PATCH (2) 28/34] strptime.3: " Alejandro Colomar
2020-09-04 15:37 ` Michael Kerrisk (man-pages)
2020-09-04 15:03 ` [PATCH (2) 29/34] tsearch.3: " Alejandro Colomar
2020-09-05 14:22 ` Michael Kerrisk (man-pages)
2020-09-04 15:04 ` [PATCH (2) 30/34] aio.7: " Alejandro Colomar
2020-09-04 17:15 ` Michael Kerrisk (man-pages)
2020-09-04 15:05 ` [PATCH (2) 31/34] fanotify.7: " Alejandro Colomar
2020-09-04 15:38 ` Michael Kerrisk (man-pages)
2020-09-04 15:06 ` [PATCH (2) 32/34] inotify.7: " Alejandro Colomar
2020-09-04 17:08 ` Michael Kerrisk (man-pages)
2020-09-04 15:07 ` [PATCH (2) 33/34] " Alejandro Colomar
2020-09-04 17:14 ` Michael Kerrisk (man-pages)
2020-09-04 15:08 ` [PATCH (2) 34/34] unix.7: " Alejandro Colomar
2020-09-04 15:12 ` Alejandro Colomar
2020-09-05 8:27 ` Michael Kerrisk (man-pages)
2020-09-05 9:37 ` Alejandro Colomar
2020-09-05 14:38 ` Michael Kerrisk (man-pages)
2020-09-05 14:52 ` Alejandro Colomar
2020-09-05 15:24 ` Michael Kerrisk (man-pages)
2020-09-04 15:40 ` Michael Kerrisk (man-pages)
2020-09-04 17:26 ` [PATCH (2) 27/34] shm_open.3: " Michael Kerrisk (man-pages)
2020-09-04 17:04 ` [PATCH (2) 26/34] rtnetlink.3: " Michael Kerrisk (man-pages)
2020-09-04 16:59 ` [PATCH (2) 25/34] " Michael Kerrisk (man-pages)
2020-09-04 16:58 ` [PATCH (2) 24/34] queue.3: " Michael Kerrisk (man-pages)
2020-09-04 15:42 ` [PATCH (2) 23/34] pthread_setaffinity_np.3: " Michael Kerrisk (man-pages)
2020-09-05 14:21 ` [PATCH (2) 22/34] pthread_create.3: " Michael Kerrisk (man-pages)
2020-09-05 14:21 ` [PATCH (2) 21/34] mbstowcs.3: " Michael Kerrisk (man-pages)
2020-09-04 15:27 ` [PATCH (2) 20/34] mbsinit.3: " Michael Kerrisk (man-pages)
2020-09-04 15:27 ` [PATCH (2) 19/34] malloc_info.3: " Michael Kerrisk (man-pages)
2020-09-04 15:25 ` [PATCH (2) 18/34] insque.3: " Michael Kerrisk (man-pages)
2020-09-04 15:25 ` [PATCH (2) 17/34] getgrouplist.3: " Michael Kerrisk (man-pages)
2020-09-04 15:21 ` [PATCH (2) 16/34] getaddrinfo.3: " Michael Kerrisk (man-pages)
2020-09-04 15:20 ` [PATCH (2) 15/34] " Michael Kerrisk (man-pages)
2020-09-04 13:54 ` Alejandro Colomar
2020-09-04 15:20 ` [PATCH (2) 14/34] cmsg.3: " Michael Kerrisk (man-pages)
2020-09-04 15:18 ` [PATCH (2) 13/34] " Michael Kerrisk (man-pages)
2020-09-04 15:17 ` [PATCH (2) 12/34] bsearch.3: " Michael Kerrisk (man-pages)
2020-09-04 15:14 ` [PATCH (2) 11/34] timerfd_create.2: " Michael Kerrisk (man-pages)
2020-09-05 8:07 ` Michael Kerrisk (man-pages)
2020-09-04 15:13 ` [PATCH (2) 10/34] signalfd.2: " Michael Kerrisk (man-pages)
2020-09-04 15:11 ` [PATCH (2) 09/34] sysctl.2: " Michael Kerrisk (man-pages)
2020-09-04 15:11 ` [PATCH (2) 08/34] poll.2: " Michael Kerrisk (man-pages)
2020-09-04 12:24 ` [PATCH (2) 06/34] perf_event_open.2: " Michael Kerrisk (man-pages)
2020-09-04 12:20 ` [PATCH (2) 05/34] open_by_handle_at.2: " Michael Kerrisk (man-pages)
2020-09-04 12:19 ` [PATCH (2) 04/34] futex.2: " Michael Kerrisk (man-pages)
2020-09-04 10:44 ` [PATCH (2) 02/34] bind.2: " Michael Kerrisk (man-pages)
2020-08-25 11:35 ` [patch] memusage.1, bind.2, eventfd.2, futex.2, open_by_handle_at.2, perf_event_open.2, poll.2, signalfd.2, sysctl.2, timerfd_create.2, bsearch.3, cmsg.3, getaddrinfo.3, getaddrinfo_a.3 getgrouplist.3, insque.3, malloc_info.3, mbsinit.3, mbstowcs.3, pthread_create.3, pthread_setaffinity_np.3, queue.3, rtnetlink.3, shm_open.3, strptime.3, tsearch.3, aio.7, fanotify.7, inotify.7, unix.7: " Michael Kerrisk (man-pages)
2020-09-05 14:20 ` [PATCH 35/35] qsort.3: " Alejandro Colomar
2020-09-05 14:23 ` Alejandro Colomar
2020-09-05 15:27 ` Michael Kerrisk (man-pages)
2020-09-05 15:32 ` Alejandro Colomar
2020-09-05 15:36 ` Michael Kerrisk (man-pages)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e1b424c2-77c1-e995-a866-67a122d7bb07@gmail.com \
--to=mtk.manpages@gmail.com \
--cc=colomar.6.4.3@gmail.com \
--cc=linux-man@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox