Re: [PATCH] cmsg.3, getaddrinfo_a.3 getgrouplist.3: Use sizeof, consistently

["Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>]

Hello Alex,

On 8/25/20 3:05 PM, Alejandro Colomar wrote:
> Hi Michael,
> 
> On 8/25/20 2:35 PM, Michael Kerrisk (man-pages) wrote:
>> Patch applied, but I had to fix up the title line, which got taken
>> from the mail subject line. Probably best to send subsequent patches
>> as new mails, rather than as replies to this thread.
> 
> This time I replied to the thread (so that the conversation can be
> followed in the archive), but changed the subject.  I hope that works.
> 
> (I sent this email only to you by accident; I'm sending it again with
> the CCs).
> 
> Patch (3) as numbered in the previous replies:
> 
> --------------------------------------------------------------------
>>From 1cb973629d94a048c5dcbe13fef76173f99dc3de Mon Sep 17 00:00:00 2001
> From: Alejandro Colomar <colomar.6.4.3@gmail.com>
> Date: Tue, 25 Aug 2020 14:52:03 +0200
> Subject: [PATCH] cmsg.3, getaddrinfo_a.3 getgrouplist.3: Use sizeof
>  consistently
> 
> Use ``sizeof`` consistently through all the examples in the following
> way:
> 
> - 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.
> 
> As a side effect, the parentheses of ``sizeof`` are not next to
> the parentheses of the whole expression, and it is visually
> easier to read.
> 
> Detailed rationale:
> 
> In C, successive multiplications are evaluated left to right (*), and
> therefore here is what happens (assuming x86_64):
> 
> ``(sizeof(x) * INT_MAX * 2)``:
> 
> 1) sizeof(x) * INT_MAX	(the type is the largest of both, which is
> 			 size_t (unsigned long; uint64_t)).
> 2) ANS * 2		(the type is again the largest: size_t)
> 
> ``(INT_MAX * 2 * sizeof(x))``:
> 
> 1) INT_MAX * 2		(the type is the largest of both, which is
> 			 int as both are int (int; int32_t), so the
> 			 result is already truncated as it doesn't fit
> 			 an int; at this point, the intermediate result
> 			 will be 2^32 - 2 (``INT_MAX - 1``) (if I did
> 			 the math right)).
> 2) ANS * 2		(the type is again the largest of both: size_t;
> 			 however, ANS was already incorrect, so the
> 			 result will be an incorrect size_t value)
> 
> (*):	https://en.cppreference.com/w/c/language/operator_precedence
> 
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>

Thanks! Patch applied.

Cheers,

Michael

> ---
>  man3/cmsg.3          | 2 +-
>  man3/getaddrinfo_a.3 | 2 +-
>  man3/getgrouplist.3  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/man3/cmsg.3 b/man3/cmsg.3
> index 2f9910093..99ee950f9 100644
> --- a/man3/cmsg.3
> +++ b/man3/cmsg.3
> @@ -242,7 +242,7 @@ cmsg = CMSG_FIRSTHDR(&msg);
>  cmsg\->cmsg_level = SOL_SOCKET;
>  cmsg\->cmsg_type = SCM_RIGHTS;
>  cmsg\->cmsg_len = CMSG_LEN(sizeof(int) * NUM_FD);
> -memcpy(CMSG_DATA(cmsg), myfds, NUM_FD * sizeof(int));
> +memcpy(CMSG_DATA(cmsg), myfds, sizeof(int) * NUM_FD);
>  .EE
>  .in
>  .SH SEE ALSO
> diff --git a/man3/getaddrinfo_a.3 b/man3/getaddrinfo_a.3
> index af8f88937..cd4cad0dc 100644
> --- a/man3/getaddrinfo_a.3
> +++ b/man3/getaddrinfo_a.3
> @@ -473,7 +473,7 @@ add_requests(void)
> 
>      while ((host = strtok(NULL, " "))) {
>          nreqs++;
> -        reqs = realloc(reqs, nreqs * sizeof(reqs[0]));
> +        reqs = realloc(reqs, sizeof(reqs[0]) * nreqs);
> 
>          reqs[nreqs \- 1] = calloc(1, sizeof(*reqs[0]));
>          reqs[nreqs \- 1]\->ar_name = strdup(host);
> diff --git a/man3/getgrouplist.3 b/man3/getgrouplist.3
> index aea52d999..372f2613f 100644
> --- a/man3/getgrouplist.3
> +++ b/man3/getgrouplist.3
> @@ -164,7 +164,7 @@ main(int argc, char *argv[])
> 
>      ngroups = atoi(argv[2]);
> 
> -    groups = malloc(ngroups * sizeof(gid_t));
> +    groups = malloc(sizeof(gid_t) * ngroups);
>      if (groups == NULL) {
>          perror("malloc");
>          exit(EXIT_FAILURE);
> 


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