Re: svn commit: r335041 - head/lib/libc/stdlib
On Wed, 13 Jun 2018, Jilles Tjoelker wrote: On Wed, Jun 13, 2018 at 08:03:13PM +1000, Bruce Evans wrote: On Wed, 13 Jun 2018, Eitan Adler wrote: Log: libc: remove explicit cast NULL in atoi There isn't any reason to cast NULL so just remove it. Noticed when cleaning up top. There are many reasons to cast NULL for all members of the ato*() family: - it is required if no prototype is in scope - C99 specifies ato*() in terms of strtol*() and uses the cast to NULL, ... - POSIX specifies ato*() in terms of strtol*() and uses the cast to NULL, ... These reasons can be summarized to a single reason: the cast is required if no prototype is in scope. I think it is unwise to call any function without a prototype in scope, since this runs a risk of undefined behaviour if you get the types wrong. That is a style matter, so it must not be enforced by specifications or man pages, and specifications and man pages should not have details to advocate it or to say when a prototype is needed for every single function. For the code in libc, we ensure a prototype is in scope and no cast is required. For the code in the man page, I doubt we should allow for programmers that play tricks by declaring system functions manually using K declarations or (even worse) call functions without declaring them at all. Programmers who understand prototypes have no difficulty with not using them. libc isn't even controlled by us. Its API is specified by the standard selected by the user. Even C99 doesn't require prototypes. Note that NULL may still need a cast when passed to a function with variable number of parameters. Ideally these types are also checked using attributes. A cast is almost needed for this NULL too. strtol()'s endptr arg should have type const char * so that strtol() can handle const char * string args without needing to cast away const in the caller. But then strtol() couldn't handle plain char * string args without needing worse casting away of const for the variable pointing to endptr. This variable would have type char * so its address would have type char **. C's type system is too weak for safe conversion of this to const char **, so the prototype is not allowed to do it without a diagnostic and an explicit cast must not be used. This cast is also dangerous, so the problem is reduced by using plain char ** for endptr. Since NULL is either an integer constant with value 0 or such a constant cast to unqualified void *, it can be converted without a diagnostic to either char ** or const char **. This is not obvious, and the explicit cast makes it clearer that conversion to plain char ** is really intended. FreeBSD used to do the same here, and should do the same here and elsewhere by copying better wording from POSIX whenever possible. For some reason, the FreeBSD text does not have the exception about error handling. This exception permits an implementation like musl's which calculates using int and hard-codes base 10, even if the compiler documents a cast from long to int as truncating bits. I don't think we should take advantage of this, though, since making atoi() faster than strtol() may encourage people to use atoi(). Encouraging use of atoi() would almost be correct. All it needs to be a good API is defined error handling for unrepresentable values and garbage input. Too bad if the programmer doesn't check for errors. Error checking for strtol() is very rarely complete or correct. Unfortunately, atoi() isn't allowed to handle garbage input correctly. It is only allowed to do the right thing for unrepresentable values. I would clamp atoi() to INT_MAX/MIN and return ERANGE for unrepresentable values instead of blindly truncating long values. This already happens accidentally with 32-bit longs. FreeBSD's man page even documents this by giving the implementation detail for the undefined behaviour. Bruce ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r335041 - head/lib/libc/stdlib
On Wed, Jun 13, 2018 at 08:03:13PM +1000, Bruce Evans wrote: > On Wed, 13 Jun 2018, Eitan Adler wrote: > > Log: > > libc: remove explicit cast NULL in atoi > > There isn't any reason to cast NULL so just remove it. Noticed when > > cleaning up top. > There are many reasons to cast NULL for all members of the ato*() family: > - it is required if no prototype is in scope > - C99 specifies ato*() in terms of strtol*() and uses the cast to NULL, >probably because this is simplest. Omitting the cast is just wrong >if no prototype is in scope. Writing the explicit cast is simpler than >writing caveats that the stated equivalence is only valid if a prototype >is in scope. > - POSIX specifies ato*() in terms of strtol*() and uses the cast to NULL, >exactly as in C99, probably because it defers to the C standard and >doesn't and doesn't risk breaking it by changing its wording except when >extending it. These reasons can be summarized to a single reason: the cast is required if no prototype is in scope. I think it is unwise to call any function without a prototype in scope, since this runs a risk of undefined behaviour if you get the types wrong. For the code in libc, we ensure a prototype is in scope and no cast is required. For the code in the man page, I doubt we should allow for programmers that play tricks by declaring system functions manually using K declarations or (even worse) call functions without declaring them at all. Note that NULL may still need a cast when passed to a function with variable number of parameters. Ideally these types are also checked using attributes. > FreeBSD used to do the same here, and should do the same here and > elsewhere by copying better wording from POSIX whenever possible. For some reason, the FreeBSD text does not have the exception about error handling. This exception permits an implementation like musl's which calculates using int and hard-codes base 10, even if the compiler documents a cast from long to int as truncating bits. I don't think we should take advantage of this, though, since making atoi() faster than strtol() may encourage people to use atoi(). -- Jilles Tjoelker ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r335041 - head/lib/libc/stdlib
On Wed, 13 Jun 2018, Eitan Adler wrote: Log: libc: remove explicit cast NULL in atoi There isn't any reason to cast NULL so just remove it. Noticed when cleaning up top. There are many reasons to cast NULL for all members of the ato*() family: - it is required if no prototype is in scope - C99 specifies ato*() in terms of strtol*() and uses the cast to NULL, probably because this is simplest. Omitting the cast is just wrong if no prototype is in scope. Writing the explicit cast is simpler than writing caveats that the stated equivalence is only valid if a prototype is in scope. - POSIX specifies ato*() in terms of strtol*() and uses the cast to NULL, exactly as in C99, probably because it defers to the C standard and doesn't and doesn't risk breaking it by changing its wording except when extending it. FreeBSD used to do the same here, and should do the same here and elsewhere by copying better wording from POSIX whenever possible. Bruce ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
svn commit: r335041 - head/lib/libc/stdlib
Author: eadler
Date: Wed Jun 13 08:52:17 2018
New Revision: 335041
URL: https://svnweb.freebsd.org/changeset/base/335041
Log:
libc: remove explicit cast NULL in atoi
There isn't any reason to cast NULL so just remove it. Noticed when
cleaning up top.
Reviewed by: pstef
Modified:
head/lib/libc/stdlib/atoi.3
head/lib/libc/stdlib/atoi.c
Modified: head/lib/libc/stdlib/atoi.3
==
--- head/lib/libc/stdlib/atoi.3 Wed Jun 13 08:52:14 2018(r335040)
+++ head/lib/libc/stdlib/atoi.3 Wed Jun 13 08:52:17 2018(r335041)
@@ -57,7 +57,7 @@ representation.
.Pp
It is equivalent to:
.Bd -literal -offset indent
-(int)strtol(nptr, (char **)NULL, 10);
+(int)strtol(nptr, NULL, 10);
.Ed
.Pp
The
Modified: head/lib/libc/stdlib/atoi.c
==
--- head/lib/libc/stdlib/atoi.c Wed Jun 13 08:52:14 2018(r335040)
+++ head/lib/libc/stdlib/atoi.c Wed Jun 13 08:52:17 2018(r335041)
@@ -46,11 +46,11 @@ __FBSDID("$FreeBSD$");
int
atoi(const char *str)
{
- return (int)strtol(str, (char **)NULL, 10);
+ return (int)strtol(str, NULL, 10);
}
int
atoi_l(const char *str, locale_t locale)
{
- return (int)strtol_l(str, (char **)NULL, 10, locale);
+ return (int)strtol_l(str, NULL, 10, locale);
}
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
