Re: [PATCH (2) 02/34] bind.2: Use sizeof consistently

Michael Kerrisk (man-pages)
2020-09-04T10:44:21Z

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/