Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expunge strtoll(3) and strtol(3) #896

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from
Draft

Expunge strtoll(3) and strtol(3) #896

wants to merge 23 commits into from

Conversation

alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Jan 9, 2024 •
edited

Remove all calls to strtoll(3) and strtol(3), and replace them by safer calls like getlong() or getnum().

Revisions

v3 v3 changes:
  • Rebase
$ git range-diff gh/rm_noneg..gh/strtoll rm_noneg..strtoll 
 1:  dcae312d =  1:  bd966bc5 lib/typetraits.h: Add macros that give information about a type
 2:  3088e310 =  2:  afea5fae src/: Use str2[u]l() instead of atoi(3)
 3:  0d3b5e68 =  3:  617570b1 lib/get_gid.c: get_gid(): Reimplement in terms of a2i()
 4:  aecd2148 =  4:  afa3cebc lib/, libsubid/, po/, src/: get_gid(): Move function to "atoi/getnum.h"
 5:  955fbeec =  5:  a83f9774 lib/: Don't open-code get_gid()
 6:  7e485fa3 =  6:  10ec7b6e lib/get_pid.c: get_pid(): Reimplement in terms of a2i()
 7:  65fd4d27 =  7:  bdf47ee8 lib/: get_pid(): Move function to "atoi/getnum.h"
 8:  3cd8b0ef =  8:  fcfb5bf8 lib/atoi/getnum.[ch]: get_fd(): Add function for parsing a file descriptor from a string
 9:  f22ef57d =  9:  cc310f6b lib/get_pid.c: get_pidfd_from_fd(): Don't open-code get_fd()
10:  72573c2d = 10:  51690f85 src/usermod.c: getulong_range(): Reimplement in terms of a2ul()
11:  a3350c44 = 11:  0de3198b lib/get_uid.c: get_uid(): Reimplement in terms of a2i()
12:  be1e25ff = 12:  5d89bf4f lib/, po/, src/: get_uid(): Move function to "atoi/getnum.h"
13:  17194b35 = 13:  16d30dfe lib/limits.c: setrlimit_value(): Reimplement in terms of a2i()
v4 v4 changes:
  • Fix const-correctness issue, which has been uncovered by the const-generic liba2i macros.
$ git range-diff rm_noneg gh/strtoll strtoll 
 1:  bd966bc5 =  1:  bd966bc5 lib/typetraits.h: Add macros that give information about a type
 2:  afea5fae =  2:  afea5fae src/: Use str2[u]l() instead of atoi(3)
 3:  617570b1 =  3:  617570b1 lib/get_gid.c: get_gid(): Reimplement in terms of a2i()
 4:  afa3cebc =  4:  afa3cebc lib/, libsubid/, po/, src/: get_gid(): Move function to "atoi/getnum.h"
 5:  a83f9774 =  5:  a83f9774 lib/: Don't open-code get_gid()
 6:  10ec7b6e =  6:  10ec7b6e lib/get_pid.c: get_pid(): Reimplement in terms of a2i()
 7:  bdf47ee8 =  7:  bdf47ee8 lib/: get_pid(): Move function to "atoi/getnum.h"
 8:  fcfb5bf8 =  8:  fcfb5bf8 lib/atoi/getnum.[ch]: get_fd(): Add function for parsing a file descriptor from a string
 9:  cc310f6b =  9:  cc310f6b lib/get_pid.c: get_pidfd_from_fd(): Don't open-code get_fd()
10:  51690f85 = 10:  51690f85 src/usermod.c: getulong_range(): Reimplement in terms of a2ul()
11:  0de3198b = 11:  0de3198b lib/get_uid.c: get_uid(): Reimplement in terms of a2i()
12:  5d89bf4f = 12:  5d89bf4f lib/, po/, src/: get_uid(): Move function to "atoi/getnum.h"
13:  16d30dfe = 13:  16d30dfe lib/limits.c: setrlimit_value(): Reimplement in terms of a2i()
 -:  -------- > 14:  cff55356 src/usermod.c: Fix const correctness

@alejandro-colomar alejandro-colomar changed the title Expunge strtoll(3) Expunge strtoll(3) and strtol(3) Jan 9, 2024
@alejandro-colomar alejandro-colomar mentioned this pull request Mar 6, 2024
Open
@alejandro-colomar
Copy link
Collaborator Author

v2 changes:

  • Rebase to master
$ git range-diff gh/rm_noneg..gh/strtoll rm_noneg..strtoll 
 1:  d987cc23 !  1:  7a754763 lib/typetraits.h: Add macros that give information about a type
    @@ Commit message
     
      ## lib/Makefile.am ##
     @@ lib/Makefile.am: libshadow_la_SOURCES = \
    -   subordinateio.c \
    -   sulog.c \
    +   time/day_to_str.c \
    +   time/day_to_str.h \
        ttytype.c \
     +  typetraits.h \
        tz.c \
 2:  04c3c1ff =  2:  35a75a2f src/: Use str2[u]l() instead of atoi(3)
 3:  901bf98e =  3:  2a5df6b4 lib/get_gid.c: get_gid(): Reimplement in terms of a2i()
 4:  15664d4c !  4:  0245884e lib/, libsubid/, po/, src/: get_gid(): Move function to "atoi/getnum.h"
    @@ lib/prototypes.h: extern int find_new_sub_gids (gid_t *range_start, unsigned lon
     
      ## lib/sgetgrent.c ##
     @@
    - #include <grp.h>
    + #include <string.h>
      
      #include "alloc.h"
     +#include "atoi/getnum.h"
    @@ lib/sgetgrent.c
     
      ## lib/sgetpwent.c ##
     @@
    - #include "defines.h"
    - #include <stdio.h>
      #include <pwd.h>
    -+
    + #include <string.h>
    + 
     +#include "atoi/getnum.h"
    + #include "defines.h"
      #include "prototypes.h"
      #include "shadowlog_internal.h"
      
 5:  e6d47f2c =  5:  637a8484 lib/: Don't open-code get_gid()
 6:  59b2f005 =  6:  e5b31b34 lib/get_pid.c: get_pid(): Reimplement in terms of a2i()
 7:  23fd5ac6 =  7:  6bd34d8c lib/: get_pid(): Move function to "atoi/getnum.h"
 8:  35b3c86c =  8:  e908ef42 lib/atoi/getnum.[ch]: get_fd(): Add function for parsing a file descriptor from a string
 9:  4e0e9bf1 =  9:  05bd8850 lib/get_pid.c: get_pidfd_from_fd(): Don't open-code get_fd()
10:  8a4f7943 = 10:  d8fa7301 src/usermod.c: getulong_range(): Reimplement in terms of a2ul()
11:  330dd287 = 11:  ae87a7e9 lib/get_uid.c: get_uid(): Reimplement in terms of a2i()
12:  c77cfdab = 12:  27692802 lib/, po/, src/: get_uid(): Move function to "atoi/getnum.h"
13:  68ba7031 = 13:  a0b322e2 lib/limits.c: setrlimit_value(): Reimplement in terms of a2i()

@alejandro-colomar
Copy link
Collaborator Author

v2b changes:

  • Rebase on master
$ git range-diff gh/rm_noneg..gh/strtoll rm_noneg..strtoll 
 1:  7a754763 =  1:  6870a0e0 lib/typetraits.h: Add macros that give information about a type
 2:  35a75a2f =  2:  ebba199f src/: Use str2[u]l() instead of atoi(3)
 3:  2a5df6b4 =  3:  a2122723 lib/get_gid.c: get_gid(): Reimplement in terms of a2i()
 4:  0245884e =  4:  6eae04d4 lib/, libsubid/, po/, src/: get_gid(): Move function to "atoi/getnum.h"
 5:  637a8484 =  5:  debb96f4 lib/: Don't open-code get_gid()
 6:  e5b31b34 =  6:  82a992d3 lib/get_pid.c: get_pid(): Reimplement in terms of a2i()
 7:  6bd34d8c =  7:  58f28b8e lib/: get_pid(): Move function to "atoi/getnum.h"
 8:  e908ef42 =  8:  5eb9fc41 lib/atoi/getnum.[ch]: get_fd(): Add function for parsing a file descriptor from a string
 9:  05bd8850 =  9:  faa7cce1 lib/get_pid.c: get_pidfd_from_fd(): Don't open-code get_fd()
10:  d8fa7301 = 10:  14ccc58b src/usermod.c: getulong_range(): Reimplement in terms of a2ul()
11:  ae87a7e9 = 11:  fad7d7bb lib/get_uid.c: get_uid(): Reimplement in terms of a2i()
12:  27692802 = 12:  30d634ba lib/, po/, src/: get_uid(): Move function to "atoi/getnum.h"
13:  a0b322e2 = 13:  97d5ce57 lib/limits.c: setrlimit_value(): Reimplement in terms of a2i()

@alejandro-colomar
Copy link
Collaborator Author

v2d changes:

  • Rebase on master
$ git range-diff gh/rm_noneg..gh/strtoll rm_noneg..strtoll 
 1:  8f9ff452 =  1:  932fa96d lib/typetraits.h: Add macros that give information about a type
 2:  413993b3 =  2:  97471e54 src/: Use str2[u]l() instead of atoi(3)
 3:  4c2159dd =  3:  a5546d76 lib/get_gid.c: get_gid(): Reimplement in terms of a2i()
 4:  dee66fc3 =  4:  8f8095bc lib/, libsubid/, po/, src/: get_gid(): Move function to "atoi/getnum.h"
 5:  c37c2ae4 =  5:  6ad6667d lib/: Don't open-code get_gid()
 6:  8dcd0354 =  6:  cc67840f lib/get_pid.c: get_pid(): Reimplement in terms of a2i()
 7:  fa78d95c =  7:  dd1413cf lib/: get_pid(): Move function to "atoi/getnum.h"
 8:  a5bb3e22 =  8:  e5ddd6cc lib/atoi/getnum.[ch]: get_fd(): Add function for parsing a file descriptor from a string
 9:  c7273620 =  9:  d66e280a lib/get_pid.c: get_pidfd_from_fd(): Don't open-code get_fd()
10:  2def551a = 10:  21ee50c4 src/usermod.c: getulong_range(): Reimplement in terms of a2ul()
11:  a41871ee = 11:  11c0777f lib/get_uid.c: get_uid(): Reimplement in terms of a2i()
12:  5f862873 = 12:  fd9f2033 lib/, po/, src/: get_uid(): Move function to "atoi/getnum.h"
13:  0247134d = 13:  ae1fd8a2 lib/limits.c: setrlimit_value(): Reimplement in terms of a2i()

@alejandro-colomar
Copy link
Collaborator Author

v2e changes:

  • Rebase
$ git range-diff gh/rm_noneg..gh/strtoll rm_noneg..strtoll 
 1:  932fa96d =  1:  cc6c9099 lib/typetraits.h: Add macros that give information about a type
 2:  97471e54 =  2:  ceb9cf25 src/: Use str2[u]l() instead of atoi(3)
 3:  a5546d76 =  3:  80b2f03b lib/get_gid.c: get_gid(): Reimplement in terms of a2i()
 4:  8f8095bc =  4:  f3b68d0f lib/, libsubid/, po/, src/: get_gid(): Move function to "atoi/getnum.h"
 5:  6ad6667d =  5:  73828d71 lib/: Don't open-code get_gid()
 6:  cc67840f =  6:  5c1f3269 lib/get_pid.c: get_pid(): Reimplement in terms of a2i()
 7:  dd1413cf =  7:  9a8e8959 lib/: get_pid(): Move function to "atoi/getnum.h"
 8:  e5ddd6cc =  8:  9c272ae7 lib/atoi/getnum.[ch]: get_fd(): Add function for parsing a file descriptor from a string
 9:  d66e280a =  9:  a079abe2 lib/get_pid.c: get_pidfd_from_fd(): Don't open-code get_fd()
10:  21ee50c4 = 10:  6b4a6d42 src/usermod.c: getulong_range(): Reimplement in terms of a2ul()
11:  11c0777f = 11:  98676974 lib/get_uid.c: get_uid(): Reimplement in terms of a2i()
12:  fd9f2033 = 12:  995d0b87 lib/, po/, src/: get_uid(): Move function to "atoi/getnum.h"
13:  ae1fd8a2 = 13:  1125c9a7 lib/limits.c: setrlimit_value(): Reimplement in terms of a2i()

@alejandro-colomar
Copy link
Collaborator Author

v2f changes:

  • Rebase
$ git range-diff gh/rm_noneg..gh/strtoll rm_noneg..strtoll 
 1:  cc6c9099 =  1:  dcae312d lib/typetraits.h: Add macros that give information about a type
 2:  ceb9cf25 =  2:  3088e310 src/: Use str2[u]l() instead of atoi(3)
 3:  80b2f03b =  3:  0d3b5e68 lib/get_gid.c: get_gid(): Reimplement in terms of a2i()
 4:  f3b68d0f =  4:  aecd2148 lib/, libsubid/, po/, src/: get_gid(): Move function to "atoi/getnum.h"
 5:  73828d71 =  5:  955fbeec lib/: Don't open-code get_gid()
 6:  5c1f3269 =  6:  7e485fa3 lib/get_pid.c: get_pid(): Reimplement in terms of a2i()
 7:  9a8e8959 =  7:  65fd4d27 lib/: get_pid(): Move function to "atoi/getnum.h"
 8:  9c272ae7 =  8:  3cd8b0ef lib/atoi/getnum.[ch]: get_fd(): Add function for parsing a file descriptor from a string
 9:  a079abe2 =  9:  f22ef57d lib/get_pid.c: get_pidfd_from_fd(): Don't open-code get_fd()
10:  6b4a6d42 = 10:  72573c2d src/usermod.c: getulong_range(): Reimplement in terms of a2ul()
11:  98676974 = 11:  a3350c44 lib/get_uid.c: get_uid(): Reimplement in terms of a2i()
12:  995d0b87 = 12:  be1e25ff lib/, po/, src/: get_uid(): Move function to "atoi/getnum.h"
13:  1125c9a7 = 13:  17194b35 lib/limits.c: setrlimit_value(): Reimplement in terms of a2i()

@alejandro-colomar
lib/atoi/a2i.[ch]: Add const-generic macros
78c27e0 
These overloaded macros allow passing either a const or a non-const
endp, and will call the appropriate function.  This kind of const
overloading has prior art in C23's string functions, such as memchr(3).

Martin suggested using an artificial function pointer in _Generic(3); it
allows switching on various types at the same time.

Link: <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf#subsubsection.7.26.5.2>
Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114731>
Co-developed-by: Martin Uecker <muecker@gwdg.de>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/getrange.c: getrange(): Use a2ul() instead of strtoul_noneg()
864c66b 
It simplifies the error checking.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/getrange.c: getrange(): Add const to pointer
3396198 
Now that we have const-generic macros, we can use a const pointer.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/getrange.c: getrange(): Add missing cast
655d06b 
isdigit(3) requires a cast if the argument is of type 'char'.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/getrange.c: getrange(): Report an ERANGE error when min>max
e8039e7 
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/gettime.c: gettime(): Call a2i() instead of strtoull_noneg()
44bf6f3 
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>
Cc: Chris Lamb <lamby@debian.org>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/atoi/strtou_noneg.[ch], tests/: strtoull_noneg(): Remove unused f…
4070b87 
…unction

All call sites were replaced by a2i() recently.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
src/check_subid_range.c: Call str2ul() instead of strtoul_noneg()
5ba384e 
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.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/atoi/strtou_noneg.[ch], tests/: strtoul_noneg(): Remove unused fu…
80ad37b 
…nction

All call sites have been replaced by functions from "atoi/a2i.h" and
"atoi/str2i.h" recently.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/typetraits.h: Add macros that give information about a type
bd966bc 
In the case of is_unsigned() and is_signed(), the natural thing would be
to compare to 0:

	#define is_unsigned(x)  (((typeof(x)) -1) > 0)
	#define is_signed(x)    (((typeof(x)) -1) < 0)

However, that would trigger -Wtype-limits, so we compare against 1,
which silences that, and does the same job.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
src/: Use str2[u]l() instead of atoi(3)
afea5fa 
atoi(3) easily triggers Undefined Behavior.  Replace it by str2[u]l(),
which are safe from that, and add type safety too.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/get_gid.c: get_gid(): Reimplement in terms of a2i()
617570b 
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/, libsubid/, po/, src/: get_gid(): Move function to "atoi/getnum.h"
afa3ceb 
Implement it as an inline function, and add restrict and ATTR_STRING()
and ATTR_ACCESS() as appropriate.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/: Don't open-code get_gid()
a83f977 
These functions were open-coding get_gid().  Use the actual function.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/get_pid.c: get_pid(): Reimplement in terms of a2i()
10ec7b6 
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/: get_pid(): Move function to "atoi/getnum.h"
bdf47ee 
Implement it as an inline function, and add restrict and ATTR_STRING()
and ATTR_ACCESS() as appropriate.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/atoi/getnum.[ch]: get_fd(): Add function for parsing a file descr…
fcfb5bf 
…iptor from a string

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/get_pid.c: get_pidfd_from_fd(): Don't open-code get_fd()
cc310f6 
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
src/usermod.c: getulong_range(): Reimplement in terms of a2ul()
51690f8 
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/get_uid.c: get_uid(): Reimplement in terms of a2i()
0de3198 
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/, po/, src/: get_uid(): Move function to "atoi/getnum.h"
5d89bf4 
Implement it as an inline function, and add restrict and ATTR_STRING()
and ATTR_ACCESS() as appropriate.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
lib/limits.c: setrlimit_value(): Reimplement in terms of a2i()
16d30df 
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented May 5, 2024 •
edited

It seems the const-generic macros have exposed some const correctness violations. They are probably nothing to worry, but I'll fix them.

Edit: Done.

@alejandro-colomar
src/usermod.c: Fix const correctness
cff5535 
Now that we use liba2i's const-generic macros, we can (and must) use a
'const char **' endp where the input string is 'const char *'.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@alejandro-colomar