Re: [PATCH (2) 02/34] bind.2: Use sizeof consistently - Michael Kerrisk (man-pages)

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/