Remove strtou[l]l_noneg(), replacing them by a2i() calls #895

alejandro-colomar · 2024-01-09T14:05:33Z

Revisions:

v3

v3 changes:

Rebase

$ git range-diff 851315ea^..gh/rm_noneg getrange..rm_noneg 
1:  851315ea = 1:  44bf6f3b lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
2:  3c3d2bb9 = 2:  4070b87d lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
3:  dc774cda = 3:  5ba384e6 src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
4:  857f2e9f = 4:  80ad37bc lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function

v3b

Rebase

$ git range-diff 44bf6f3b^..gh/rm_noneg shadow/master..rm_noneg 
1:  44bf6f3b = 1:  7de6994c lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
2:  4070b87d = 2:  1ade434f lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
3:  5ba384e6 = 3:  ca8d63c0 src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
4:  80ad37bc = 4:  110447bb lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function

v3c

Reword error message

$ git range-diff shadow/master gh/rm_noneg rm_noneg 
1:  7de6994c ! 1:  368df2d2 lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
    @@ lib/gettime.c
     -          return epoch;
     +  if (a2i(time_t, &epoch, source_date_epoch, NULL, 10, 0, fallback) == -1) {
     +          fprintf(shadow_logfd,
    -+                  _("Environment variable $SOURCE_DATE_EPOCH: strtoi(\"%s\"): %s"),
    ++                  _("Environment variable $SOURCE_DATE_EPOCH: a2i(\"%s\"): %s"),
     +                  source_date_epoch, strerror(errno));
     +          return fallback;
        }
2:  1ade434f = 2:  7e8aa6e1 lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
3:  ca8d63c0 = 3:  ce1c306b src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
4:  110447bb = 4:  fbe6d9bf lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function

v3d

Rebase
Reviewed-by @ikerexxe

$ git range-diff 368df2d2^..gh/rm_noneg shadow/master..rm_noneg 
1:  368df2d2 ! 1:  9bb6397c lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
    @@ Commit message
         (except the value of 'fallback').
     
         Link: <https://github.com/shadow-maint/shadow/commit/cb610d54b47ea2fc3da5a1b7c5a71274ada91371#r136407772>
    +    Reviewed-by: Iker Pedrosa <[email protected]>
         Cc: Chris Lamb <[email protected]>
         Cc: Serge Hallyn <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
2:  7e8aa6e1 ! 2:  19cc0dfe lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
    @@ Commit message
     
         All call sites were replaced by a2i() recently.
     
    +    Reviewed-by: Iker Pedrosa <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/atoi/strtou_noneg.c ##
3:  ce1c306b ! 3:  e189b6ec src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
    @@ Commit message
         A consequence of this change is that the program now accepts numbers in
         bases 8 and 16.  That's not a problem here, I think.
     
    +    Reviewed-by: Iker Pedrosa <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## src/check_subid_range.c ##
4:  fbe6d9bf ! 4:  15af2f3b lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function
    @@ Commit message
         All call sites have been replaced by functions from "atoi/a2i.h" and
         "atoi/str2i.h" recently.
     
    +    Reviewed-by: Iker Pedrosa <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/atoi/strtou_noneg.c ##

alejandro-colomar · 2024-03-15T00:57:03Z

v2 changes:

Rebase to master

$ git range-diff gh/getrange..gh/rm_noneg getrange..rm_noneg 
1:  9f8efae5 = 1:  e8aadb9c lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
2:  22b425f8 = 2:  80f9e237 lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
3:  eb1c2eaf = 3:  5959165f src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
4:  b23f9c03 = 4:  4c751792 lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function

alejandro-colomar · 2024-03-21T14:30:06Z

v2b changes:

Rebase on master

$ git range-diff gh/getrange..gh/rm_noneg getrange..rm_noneg 
1:  e8aadb9c = 1:  9ecdce44 lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
2:  80f9e237 = 2:  f71fbb3c lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
3:  5959165f = 3:  28ace782 src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
4:  4c751792 = 4:  096cce6a lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function

alejandro-colomar · 2024-03-23T23:30:27Z

v2c changes:

Rebase on master

$ git range-diff gh/getrange..gh/rm_noneg getrange..rm_noneg 
1:  9ecdce44 = 1:  adaf6782 lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
2:  f71fbb3c = 2:  870b9eb5 lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
3:  28ace782 = 3:  669e57b8 src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
4:  096cce6a = 4:  8fb1abfd lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function

alejandro-colomar · 2024-03-29T19:39:59Z

v2d changes:

Rebase on master

$ git range-diff gh/getrange..gh/rm_noneg getrange..rm_noneg 
1:  adaf6782 = 1:  e63f5270 lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
2:  870b9eb5 = 2:  f52712c2 lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
3:  669e57b8 = 3:  12d987fb src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
4:  8fb1abfd = 4:  07e26fe7 lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function

alejandro-colomar · 2024-04-16T01:58:05Z

v2e changes:

Rebase

$ git range-diff e63f5270^..gh/rm_noneg getrange..rm_noneg 
1:  e63f5270 = 1:  940e0993 lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
2:  f52712c2 = 2:  728c5f45 lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
3:  12d987fb = 3:  014f107e src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
4:  07e26fe7 = 4:  eb1290de lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function

alejandro-colomar · 2024-04-16T13:51:58Z

v2f changes:

Rebase (the problem has been workarounded in getrange).

$ git range-diff gh/getrange..gh/rm_noneg getrange..rm_noneg 
1:  940e0993 = 1:  851315ea lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
2:  728c5f45 = 2:  3c3d2bb9 lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused function
3:  014f107e = 3:  dc774cda src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
4:  eb1290de = 4:  857f2e9f lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused function

alejandro-colomar · 2024-05-17T13:59:32Z

@lamby
We've merge all the PRs that were dependencies of this one, so I expect this to be merged soon. This one includes the fix for the problems we discussed.

alejandro-colomar · 2024-05-17T18:45:01Z

lib/gettime.c

-	} else {
-		/* Valid */
-		return epoch;
+	if (a2i(time_t, &epoch, source_date_epoch, NULL, 10, 0, fallback) == -1) {


@lamby Should we accept negative values? Why did this use strtoull(1) originally?

I'm blocked until this questions is answered. Feel free to close it and merge the PR at your convenience

Thanks! I'm merging.

alejandro-colomar · 2024-05-21T16:28:00Z

Regardless of @lamby responding my doubts, this is ready for review. @hallyn, @ikerexxe

ikerexxe

Comment inline

lib/gettime.c

src/check_subid_range.c

ikerexxe

LGTM! Thanks for the patches and the explanations.

time_t isn't necessarily unsigned (in fact, it's likely to be signed. Therefore, parse the number as the right type, via a2i(time_t, ...). Still, reject negative numbers, just to be cautious. It was done before (strtoull_noneg()), so it shouldn't be a problem. (However, strtoull_noneg() was only introduced recently, and before that we called strtoull(3), which silently accepted negative values.) Remove the limitation of ULONG_MAX, which seems arbitrary. It probably was written in times where 'time_t' had the same length of 'long', and this was thus a test that the value didn't overflow 'time_t'. Such a test is implicit in the a2i() call, so forget about it. Unify the error messages into a single one that provides all the info (except the value of 'fallback'). Link: <shadow-maint@cb610d5#r136407772> Reviewed-by: Iker Pedrosa <[email protected]> Cc: Chris Lamb <[email protected]> Cc: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

…unction All call sites were replaced by a2i() recently. Reviewed-by: Iker Pedrosa <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

It is a simpler call, with more type safety. A consequence of this change is that the program now accepts numbers in bases 8 and 16. That's not a problem here, I think. Reviewed-by: Iker Pedrosa <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

…nction All call sites have been replaced by functions from "atoi/a2i.h" and "atoi/str2i.h" recently. Reviewed-by: Iker Pedrosa <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

alejandro-colomar force-pushed the rm_noneg branch 7 times, most recently from c21f32d to bce02f8 Compare January 16, 2024 11:41

alejandro-colomar mentioned this pull request Jan 16, 2024

Reject negative numbers in strtoul(3) #875

Merged

alejandro-colomar force-pushed the rm_noneg branch from bce02f8 to 87dfdbd Compare January 17, 2024 01:29

alejandro-colomar changed the title ~~Remove strtou[l]l_noneg(), as they've been replaced by getlong() variants~~ Remove strtou[l]l_noneg(), as they've been replaced by a2i() variants Jan 17, 2024

alejandro-colomar changed the title ~~Remove strtou[l]l_noneg(), as they've been replaced by a2i() variants~~ Remove strtou[l]l_noneg(), as they've been replaced by a2i() calls Jan 17, 2024

alejandro-colomar force-pushed the rm_noneg branch 3 times, most recently from ccf1910 to 5e0df30 Compare January 21, 2024 00:40

alejandro-colomar force-pushed the rm_noneg branch from 5e0df30 to 143eda5 Compare January 29, 2024 15:19

alejandro-colomar force-pushed the rm_noneg branch from 143eda5 to d86f557 Compare February 6, 2024 01:20

alejandro-colomar force-pushed the rm_noneg branch 2 times, most recently from 9391bf2 to 4015fd5 Compare February 20, 2024 00:08

alejandro-colomar mentioned this pull request Mar 6, 2024

gettime() rejects >ULONG_MAX (Reproducible Builds problem) #961

Open

alejandro-colomar force-pushed the rm_noneg branch 3 times, most recently from b23f9c0 to 4c75179 Compare March 15, 2024 00:56

alejandro-colomar force-pushed the rm_noneg branch from 4c75179 to 096cce6 Compare March 21, 2024 14:29

alejandro-colomar force-pushed the rm_noneg branch from 096cce6 to 8fb1abf Compare March 23, 2024 23:30

alejandro-colomar force-pushed the rm_noneg branch from 8fb1abf to 07e26fe Compare March 29, 2024 19:39

alejandro-colomar force-pushed the rm_noneg branch from 07e26fe to eb1290d Compare April 16, 2024 01:57

This comment was marked as resolved.

Sign in to view

alejandro-colomar force-pushed the rm_noneg branch from eb1290d to 857f2e9 Compare April 16, 2024 13:51

alejandro-colomar force-pushed the rm_noneg branch from 857f2e9 to 80ad37b Compare May 5, 2024 22:17

alejandro-colomar force-pushed the rm_noneg branch from 80ad37b to 110447b Compare May 17, 2024 13:55

alejandro-colomar marked this pull request as ready for review May 17, 2024 13:58

alejandro-colomar changed the title ~~Remove strtou[l]l_noneg(), as they've been replaced by a2i() calls~~ Remove strtou[l]l_noneg(), replacing them by a2i() calls May 17, 2024

alejandro-colomar force-pushed the rm_noneg branch from 110447b to fbe6d9b Compare May 17, 2024 14:44

alejandro-colomar commented May 17, 2024

View reviewed changes

alejandro-colomar requested review from hallyn and ikerexxe May 21, 2024 16:28

ikerexxe reviewed May 22, 2024

View reviewed changes

lib/gettime.c Show resolved Hide resolved

src/check_subid_range.c Show resolved Hide resolved

ikerexxe approved these changes May 27, 2024

View reviewed changes

alejandro-colomar added 4 commits May 27, 2024 16:29

lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused f…

19cc0df

…unction All call sites were replaced by a2i() recently. Reviewed-by: Iker Pedrosa <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused fu…

15af2f3

…nction All call sites have been replaced by functions from "atoi/a2i.h" and "atoi/str2i.h" recently. Reviewed-by: Iker Pedrosa <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

alejandro-colomar force-pushed the rm_noneg branch from fbe6d9b to 15af2f3 Compare May 27, 2024 14:30

alejandro-colomar merged commit 71e2835 into shadow-maint:master May 27, 2024
8 checks passed

alejandro-colomar deleted the rm_noneg branch May 27, 2024 16:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove strtou[l]l_noneg(), replacing them by a2i() calls #895

Remove strtou[l]l_noneg(), replacing them by a2i() calls #895

alejandro-colomar commented Jan 9, 2024 •

edited

Loading

alejandro-colomar commented Mar 15, 2024

alejandro-colomar commented Mar 21, 2024

alejandro-colomar commented Mar 23, 2024

alejandro-colomar commented Mar 29, 2024

alejandro-colomar commented Apr 16, 2024

This comment was marked as resolved.

alejandro-colomar commented Apr 16, 2024

alejandro-colomar commented May 17, 2024

alejandro-colomar May 17, 2024 •

edited

Loading

ikerexxe May 27, 2024

alejandro-colomar May 27, 2024

alejandro-colomar commented May 21, 2024

ikerexxe left a comment

ikerexxe left a comment

Remove strtou[l]l_noneg(), replacing them by a2i() calls #895

Remove strtou[l]l_noneg(), replacing them by a2i() calls #895

Conversation

alejandro-colomar commented Jan 9, 2024 • edited Loading

alejandro-colomar commented Mar 15, 2024

alejandro-colomar commented Mar 21, 2024

alejandro-colomar commented Mar 23, 2024

alejandro-colomar commented Mar 29, 2024

alejandro-colomar commented Apr 16, 2024

This comment was marked as resolved.

alejandro-colomar commented Apr 16, 2024

alejandro-colomar commented May 17, 2024

alejandro-colomar May 17, 2024 • edited Loading

Choose a reason for hiding this comment

ikerexxe May 27, 2024

Choose a reason for hiding this comment

alejandro-colomar May 27, 2024

Choose a reason for hiding this comment

alejandro-colomar commented May 21, 2024

ikerexxe left a comment

Choose a reason for hiding this comment

ikerexxe left a comment

Choose a reason for hiding this comment

alejandro-colomar commented Jan 9, 2024 •

edited

Loading

alejandro-colomar May 17, 2024 •

edited

Loading