Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 9 Aug 2014 10:52:45 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Sean Bruno <sbruno@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r269741 - head/sys/boot/userboot/userboot
Message-ID:  <20140809101837.O1460@besplex.bde.org>
In-Reply-To: <53e54583.2353.2c317825@svn.freebsd.org>
References:  <53e54583.2353.2c317825@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 8 Aug 2014, Sean Bruno wrote:

> Log:
>  Quiesce warning about discarding a const qualifier in assignement.

This replaces an error by another error.  (Non-C compilers like clang
and gcc give only a warning for the first error and not even a warning
for the second error when it is quiesced.)

> Modified: head/sys/boot/userboot/userboot/devicename.c
> ==============================================================================
> --- head/sys/boot/userboot/userboot/devicename.c	Fri Aug  8 21:27:33 2014	(r269740)
> +++ head/sys/boot/userboot/userboot/devicename.c	Fri Aug  8 21:47:47 2014	(r269741)
> @@ -91,7 +91,7 @@ userboot_parsedev(struct disk_devdesc **
>     struct disk_devdesc *idev;
>     struct devsw	*dv;
>     int			i, unit, err;
> -    char		*cp;
> +    const char		*cp;
>     const char		*np;
>
>     /* minimum length check */

This code also has bad style, with unsorted declarations and
indentation in all the wrong places.  Local declarations are not
indented in KNF, but are excessively indented here.  Statements
are Indented by 1 tab in KNF, but by only 4 spaces here.

> @@ -126,7 +126,7 @@ userboot_parsedev(struct disk_devdesc **
> 	unit = 0;
>
> 	if (*np && (*np != ':')) {
> -	    unit = strtol(np, &cp, 0);	/* get unit number if present */
> +	    unit = strtol(np, (char **)&cp, 0);	/* get unit number if present */
> 	    if (cp == np) {
> 		err = EUNIT;
> 		goto fail;

A C compiler (TenDRA) says:

"userboot.c", line 129: Error:
   [ISO C90 6.3.2.2]: In call of function 'strtol'.
   [ISO C90 6.1.2.6]: The types 'const char *' and 'char *' are incompatible.
   [ISO C90 6.3.4]: Types in pointer conversion should be compatible.
   [ISO C90 6.3.16]: Can't perform this conversion by assignment.
   [ISO C90 6.3.2.2]: Argument 2 is converted to parameter type.

This is a well known problem in the strtol() API.  endptr in it cannot
have const qualifiers, and even bogus casts to remove the qualifiers are
errors.  In the above, you applied a bogus cast to break a warning that
should be an error, but the cast itself is an error.

Correct code:

 	char *xcp;
 	...
 	unit = strtol(np, &xcp, 0);	/* banal comment deleted */
 	cp = xcp;

strtol() must be passed a char **, not something else bogusly cast to
break the warning that should be an error.  Then it returns a char *
(in xcp here).  This can be assigned to a const char * with no problems.
One reason strtol() takes doesn't take consts in its endptr is that
if it returned a const char * then you couldn't assign it to a char *
without problems.

Note that calling strtol() reduces type safety, but no worse than
strchr().  You start with a const char * like np and get back a plain
char * like xcp.  Assigning to *xcp would be an error.  The only clearly
safe use of the result is to assign xcp it to a const char * like cp
above before using it.  This is only needed when np is const char *
of course.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140809101837.O1460>