public inbox for linux-man@vger.kernel.org
 help / color / mirror / Atom feed
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,
	Jakub Wilk <jwilk@jwilk.net>
Subject: Re: [PATCH (2) 02/34] bind.2: Use sizeof consistently
Date: Fri, 4 Sep 2020 12:44:14 +0200	[thread overview]
Message-ID: <7034126c-ecac-8a3c-bd22-6d28204c0839@gmail.com> (raw)
In-Reply-To: <d38d3644-698c-41e4-4d6a-5d999c0a3fdd@gmail.com>

Hello Alex,

On 9/4/20 12:19 PM, Alejandro Colomar wrote:
> Hello Michael,
> 
> On 9/4/20 10:20 AM, Michael Kerrisk (man-pages) wrote:
>> I must admit that I don't care too much either way on this.
>> That is to say, I'm not sure one way is any clearer than
>> the other. However, I have applied the patch.
> 
> There are places where I wouldn't say there are any readability
> benefits.  However, there are functions such as malloc() or memset(),
> where using the type could lead to future bugs, so IMHO it's better to
> just be consistent and use always the name, unless there are clear
> readability problems (or other problems).
> 
> In the end, someone thought it to be important enough to write it in the
> kernel coding style.
> 
> I don't expect all of these patches to be applied, as I had doubts when
> writing some of them, but we can discuss those where it is better to
> keep the type.
> 
>>
>> In passing, I note that there is a clarity issue that I do
>> find more significant though: the repeated calculations in
>> the malloc() and printf() calls. So I changed that:
>>
> https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=15fc4aab1f22c2d4f62ab7f74bbb844942708633

Patch applied.

Cheers,

Michael


> ------------------------------------------------------------------------
>>From 54016160b603454fbe4f38d6a81886a03fe2ffdf Mon Sep 17 00:00:00 2001
> From: Alejandro Colomar <colomar.6.4.3@gmail.com>
> Date: Thu, 3 Sep 2020 21:24:43 +0200
> Subject: [PATCH 02/34] bind.2: Use sizeof consistently
> 
> Use ``sizeof`` consistently through all the examples in the following
> way:
> 
> - Use the name of the variable instead of its type as argument for
>   ``sizeof``.
> 
> 	Rationale:
> 	https://www.kernel.org/doc/html/v5.8/process/coding-style.html#allocating-memory
> 
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
> ---
>  man2/bind.2 | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/man2/bind.2 b/man2/bind.2
> index 72aac9555..74e34b6bd 100644
> --- a/man2/bind.2
> +++ b/man2/bind.2
> @@ -293,14 +293,14 @@ main(int argc, char *argv[])
>      if (sfd == \-1)
>          handle_error("socket");
> 
> -    memset(&my_addr, 0, sizeof(struct sockaddr_un));
> +    memset(&my_addr, 0, sizeof(my_addr));
>                          /* Clear structure */
>      my_addr.sun_family = AF_UNIX;
>      strncpy(my_addr.sun_path, MY_SOCK_PATH,
>              sizeof(my_addr.sun_path) \- 1);
> 
>      if (bind(sfd, (struct sockaddr *) &my_addr,
> -            sizeof(struct sockaddr_un)) == \-1)
> +            sizeof(my_addr)) == \-1)
>          handle_error("bind");
> 
>      if (listen(sfd, LISTEN_BACKLOG) == \-1)
> @@ -309,7 +309,7 @@ main(int argc, char *argv[])
>      /* Now we can accept incoming connections one
>         at a time using accept(2) */
> 
> -    peer_addr_size = sizeof(struct sockaddr_un);
> +    peer_addr_size = sizeof(peer_addr);
>      cfd = accept(sfd, (struct sockaddr *) &peer_addr,
>                   &peer_addr_size);
>      if (cfd == \-1)
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

  parent reply	other threads:[~2020-09-04 10:44 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)
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                         ` Michael Kerrisk (man-pages) [this message]
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=7034126c-ecac-8a3c-bd22-6d28204c0839@gmail.com \
    --to=mtk.manpages@gmail.com \
    --cc=colomar.6.4.3@gmail.com \
    --cc=jwilk@jwilk.net \
    --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