[PATCH v4 00/10] strtol(3)-related fixes

[Alejandro Colomar]

Hi Branden, Dave,

In this v4, I've found another case of the LONG_MAX==INT_MAX problem.
I've also added two authors of bugs being fixed to CC, in case they've
written similar bugs in other software and they want to fix them.  Below
is a range diff.

I strongly suggest considering the use of liba2i.  Other APIs simply
suck:

        atoi(3)
                No way to prevent UB (fault of ISO C, not of the API).

        strtol(3)
                It has a long history of misuse.  I guess this patch set
                is proof of it.  If you want more of it, perhaps in a
                setuid project, you'll want to check my 60-commit long
                patch set to shadow:
                <https://github.com/shadow-maint/shadow/pull/893>.

        strtonum(3) (OpenBSD, NetBSD, libbsd)
                This API is not generic.  It doesn't serve all the use
                cases that the other APIs support.

        strtoi(3) (NetBSD, libbsd)
                This API had a bug until January of this year, when I
                fixed it.  It's a relatively good API, but still can be
                improved a little bit.  If you want to avoid liba2i for
                being too recent, this is the other option that I
                consider reasonable.

        xstrtol() (gnulib)
                What crap is this?  It's not even documented?

        a2i(3) (liba2i)
                Based on strtoi(3), with added type safety, and no known
                bugs (admittedly, it's too new for that, but hey, it's
                just a thin wrapper around strtoi(3), which I fixed
                recently).

Have a lovely day!
Alex


Alejandro Colomar (10):
  [libgroff]: Remove redundant checks.
  [libgroff]: Remove dead code
  src/: Remove redundant checks after strtol(3).
  [grolbp]: Remove bogus (and redundant) check
  src/: ceil_prime(): Add function to get the lowest prime not less than
    n
  [indxbib]: Don't else after [[noreturn]]
  [indxbib]: Clear errno before calling strtol(3)
  [indxbib]: Remove dead code
  [indxbib]: Collapse related tests
  [grolbp]: Fix range check after strtol(3)

 src/devices/grodvi/dvi.cpp      |  4 ++--
 src/devices/grolbp/lbp.cpp      |  7 ++++---
 src/devices/grolj4/lj4.cpp      |  6 +++---
 src/devices/grops/ps.cpp        |  4 ++--
 src/devices/grops/psrm.cpp      |  2 +-
 src/include/lib.h               |  2 +-
 src/libs/libbib/index.cpp       |  4 +---
 src/libs/libgroff/curtime.cpp   |  3 +--
 src/libs/libgroff/font.cpp      |  2 +-
 src/libs/libgroff/prime.cpp     | 18 +++++++++++++++++-
 src/preproc/eqn/lex.cpp         |  2 +-
 src/preproc/pic/tex.cpp         |  2 +-
 src/preproc/refer/command.cpp   |  3 +--
 src/preproc/refer/ref.cpp       |  2 +-
 src/preproc/refer/refer.cpp     |  6 +++---
 src/utils/indxbib/indxbib.cpp   | 27 ++++++++-------------------
 src/utils/lkbib/lkbib.cpp       |  2 +-
 src/utils/lookbib/lookbib.cpp   |  2 +-
 src/utils/tfmtodit/tfmtodit.cpp |  5 +----
 19 files changed, 51 insertions(+), 52 deletions(-)

Range-diff against v3:
 1:  1ade0b95a !  1:  e79b884b4 [libgroff]: Remove redundant checks.
    @@ Commit message
     
         ERANGE can only happen if strtol(3) returns either LONG_MIN or 
LONG_MAX.
     
    +    Fixes: e4290210f2c1 ("Implement `SOURCE_DATE_EPOCH' for reproducible 
builds.")
         Link: <https://savannah.gnu.org/bugs/?65451>
         Cc: "G. Branden Robinson" <branden@debian.org>
         Cc: Dave Kemper <saint.snit@gmail.com>
         Cc: "James K. Lowden" <jklowden@schemamania.org>
    +    Cc: Colin Watson <cjwatson@debian.org>
    +    Cc: Werner LEMBERG <wl@gnu.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/libs/libgroff/curtime.cpp ##
 2:  b00a34743 !  2:  0f7a1dbae [libgroff]: Remove dead code
    @@ Commit message
     
         strtol(3) can only report ERANGE, if the base is valid (and it is).
     
    +    Fixes: e4290210f2c1 ("Implement `SOURCE_DATE_EPOCH' for reproducible 
builds.")
         Link: <https://savannah.gnu.org/bugs/?65451>
         Cc: "G. Branden Robinson" <branden@debian.org>
         Cc: Dave Kemper <saint.snit@gmail.com>
         Cc: "James K. Lowden" <jklowden@schemamania.org>
    +    Cc: Colin Watson <cjwatson@debian.org>
    +    Cc: Werner LEMBERG <wl@gnu.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/libs/libgroff/curtime.cpp ##
 3:  b16590405 !  3:  7b64b59d3 src/: Remove redundant checks after strtol(3).
    @@ Commit message
         Cc: "G. Branden Robinson" <branden@debian.org>
         Cc: Dave Kemper <saint.snit@gmail.com>
         Cc: "James K. Lowden" <jklowden@schemamania.org>
    +    Cc: Colin Watson <cjwatson@debian.org>
    +    Cc: Werner LEMBERG <wl@gnu.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/devices/grodvi/dvi.cpp ##
 4:  be068e3e4 !  4:  f61b16e5e [grolbp]: Remove bogus (and redundant) check
    @@ Commit message
         Cc: "G. Branden Robinson" <branden@debian.org>
         Cc: Dave Kemper <saint.snit@gmail.com>
         Cc: "James K. Lowden" <jklowden@schemamania.org>
    +    Cc: Colin Watson <cjwatson@debian.org>
    +    Cc: Werner LEMBERG <wl@gnu.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/devices/grolbp/lbp.cpp ##
 5:  3157b9dd4 !  5:  647391522 src/: ceil_prime(): Add function to get the 
lowest prime not smaller than n
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    src/: ceil_prime(): Add function to get the lowest prime not smaller 
than n
    +    src/: ceil_prime(): Add function to get the lowest prime not less than 
n
     
         And use it where the same logic was being open-coded.
     
    @@ Commit message
         Cc: "G. Branden Robinson" <branden@debian.org>
         Cc: Dave Kemper <saint.snit@gmail.com>
         Cc: "James K. Lowden" <jklowden@schemamania.org>
    +    Cc: Colin Watson <cjwatson@debian.org>
    +    Cc: Werner LEMBERG <wl@gnu.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/include/lib.h ##
 6:  f51a4b177 !  6:  dd5d8b1c0 [indxbib]: Don't else after [[noreturn]]
    @@ Commit message
         Cc: "G. Branden Robinson" <branden@debian.org>
         Cc: Dave Kemper <saint.snit@gmail.com>
         Cc: "James K. Lowden" <jklowden@schemamania.org>
    +    Cc: Colin Watson <cjwatson@debian.org>
    +    Cc: Werner LEMBERG <wl@gnu.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/utils/indxbib/indxbib.cpp ##
 7:  0212f9790 !  7:  2ea8b950b [indxbib]: Clear errno before calling strtol(3)
    @@ Commit message
         Cc: "G. Branden Robinson" <branden@debian.org>
         Cc: Dave Kemper <saint.snit@gmail.com>
         Cc: "James K. Lowden" <jklowden@schemamania.org>
    +    Cc: Colin Watson <cjwatson@debian.org>
    +    Cc: Werner LEMBERG <wl@gnu.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/utils/indxbib/indxbib.cpp ##
 8:  2bc3edd6a !  8:  6a02a0b3b [indxbib]: Remove dead code
    @@ Commit message
         Cc: "G. Branden Robinson" <branden@debian.org>
         Cc: Dave Kemper <saint.snit@gmail.com>
         Cc: "James K. Lowden" <jklowden@schemamania.org>
    +    Cc: Colin Watson <cjwatson@debian.org>
    +    Cc: Werner LEMBERG <wl@gnu.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/utils/indxbib/indxbib.cpp ##
 9:  06d68f407 !  9:  dd7d94b19 [indxbib]: Collapse related tests
    @@ Commit message
         Cc: "G. Branden Robinson" <branden@debian.org>
         Cc: Dave Kemper <saint.snit@gmail.com>
         Cc: "James K. Lowden" <jklowden@schemamania.org>
    +    Cc: Colin Watson <cjwatson@debian.org>
    +    Cc: Werner LEMBERG <wl@gnu.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## src/utils/indxbib/indxbib.cpp ##
 -:  --------- > 10:  e19b783c0 [grolbp]: Fix range check after strtol(3)
-- 
2.43.0

signature.asc
Description: PGP signature