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@xxxxxxxxx> 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@xxxxxxxxx> --- 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); -- 2.28.0
