Re: Warnings about error_t in glibc on GNU/Hurd

[Thomas Schwinge]

Hi!

On Wed, 29 May 2013 15:53:24 -0700 (PDT), Roland McGrath  
wrote:
> > +   /* Avoid warning: case value '0' not in enumerated type 'error_t'.  */
> > +   ESUCCESS = 0,
> 
> I don't think this is the right comment.  If anybody reads this comment at
> all, that doesn't tell them much.  Perhaps something like:
> 
> /* The value zero always means success and it is perfectly fine for
>code to use 0 explicitly (or implicitly, e.g. via Boolean coercion).
>Having an enum entry for zero both makes the debugger print the name
>for error_t-typed zero values, and prevents the compiler from issuing
>warnings about 'case 0:' in a switch on an error_t-typed value.  */

Thanks; pushed as commit 883359805325a82f4f291ff85624f141f6c93636 using
your above words.


Grüße,
 Thomas


pgpryw_8XOONT.pgp
Description: PGP signature

Re: Warnings about error_t in glibc on GNU/Hurd

[Roland McGrath]

> I'd suggest to go with the simple ESUCCESS instead of spending some more
> months ;-) thinking about a "more suitable" name.  

That is indisputably reasonable, but I did just find myself spending a few
minutes thinking about what name would be more suitable. ;-)

> + /* Avoid warning: case value '0' not in enumerated type 'error_t'.  */
> + ESUCCESS = 0,

I don't think this is the right comment.  If anybody reads this comment at
all, that doesn't tell them much.  Perhaps something like:

/* The value zero always means success and it is perfectly fine for
   code to use 0 explicitly (or implicitly, e.g. via Boolean coercion).
   Having an enum entry for zero both makes the debugger print the name
   for error_t-typed zero values, and prevents the compiler from issuing
   warnings about 'case 0:' in a switch on an error_t-typed value.  */

Re: Warnings about error_t in glibc on GNU/Hurd

[Thomas Schwinge]

Hi!

On Tue, 24 Jul 2012 11:39:02 +0200, I wrote:
> On Mon, 23 Jul 2012 21:29:42 -0400, Barry deFreese  
> wrote:
> > How about ENOERR?? :)  Sorry, couldn't resist.
> 
> If going that route, I'd prefer ESUCCESS -- in spirit of Mach's
> KERN_SUCCESS, D_SUCCESS (devices), ERR_SUCCESS (from , of
> type mach_error_t, also known as err_none -- but neither of which is used
> anywhere).

(And MACH_MSG_SUCCESS (which used in a few places in glibc in »switch
((error_t) err)« statements) also already is #defined with code 0, but
not added into the error_t enum.)

> > BTW, Thomas has a listing of several of the errors/warnings listed here:

.

> > On 7/23/2012 3:51 PM, Roland McGrath wrote:
> > > Grepping for 'case 0:' shows exactly 8 places where this warning
> > > should arise.  One of those is the __hurd_fail inline in hurd.h, so
> > > that one instance probably generates the warning in many
> > > compilations.
> 
> Correct.

> > > I'm not real sure about giving an E* name to 0, since E* names are
> > > for error codes, and 0 isn't one.  But OTOH for error_t 0 is
> > > certainly one of the expected values, so it makes sense to have an
> > > enum constant for it.  The question is what name to use, and then
> > > whether to actually use that name or just add it to suppress the
> > > warning while still using 0 in the code.

> > > Assuming the warning behavior is consistent with what I've seen, my
> > > inclination is not to change the code to use a symbolic name for
> > > this.  That's mostly because the obvious choices like ESUCCESS or
> > > EOK just seem wrong to me since the E* name space ought to be for
> > > actual error codes, and I can't think of a good name I'd actually
> > > want to be writing in the code.  So I'm inclined to pick some name
> > > that is not very friendly to use (i.e. long and verbose), perhaps
> > > even in the _* space, and add that to the error_t enum just for the
> > > purpose of suppressing these warnings and not expect people to use
> > > it in the code.
> 
> That could be done in sysdeps/mach/hurd/errnos.awk:BEGIN to avoid having
> to go through the manual.

I'd suggest to go with the simple ESUCCESS instead of spending some more
months ;-) thinking about a "more suitable" name.  Here is the patch I
tested to fix all these issues; OK to commit?

* sysdeps/mach/hurd/errnos.awk (BEGIN): Emit ESUCCESS.
* sysdeps/mach/hurd/bits/errno.h: Regenerate.

diff --git a/sysdeps/mach/hurd/bits/errno.h b/sysdeps/mach/hurd/bits/errno.h
index 3b6fe76..635cfb3 100644
--- a/sysdeps/mach/hurd/bits/errno.h
+++ b/sysdeps/mach/hurd/bits/errno.h
@@ -9,6 +9,9 @@
 
 enum __error_t_codes
 {
+   /* Avoid warning: case value '0' not in enumerated type 'error_t'.  */
+   ESUCCESS = 0,
+
 #undef EDOM
 #undef ERANGE
EPERM   = _HURD_ERRNO (1),
diff --git a/sysdeps/mach/hurd/errnos.awk b/sysdeps/mach/hurd/errnos.awk
index 9748e6e..cfff09c 100644
--- a/sysdeps/mach/hurd/errnos.awk
+++ b/sysdeps/mach/hurd/errnos.awk
@@ -32,6 +32,9 @@ BEGIN {
 print "";
 print "#ifdef _ERRNO_H\n";
 print "enum __error_t_codes\n{";
+print "\t/* Avoid warning: case value '0' not in enumerated type 
'error_t'.  */";
+print "\tESUCCESS = 0,"
+print "";
 errnoh = 0;
 maxerrno = 0;
 in_mach_errors = "";


Grüße,
 Thomas


pgpI4JPfZKBXY.pgp
Description: PGP signature

Re: Warnings about error_t in glibc on GNU/Hurd

[Thomas Schwinge]

Hi!

On Mon, 23 Jul 2012 21:29:42 -0400, Barry deFreese  wrote:
> How about ENOERR?? :)  Sorry, couldn't resist.

If going that route, I'd prefer ESUCCESS -- in spirit of Mach's
KERN_SUCCESS, D_SUCCESS (devices), ERR_SUCCESS (from , of
type mach_error_t, also known as err_none -- but neither of which is used
anywhere).

> BTW, Thomas has a listing of several of the errors/warnings listed here:

See here:
.

> On 7/23/2012 3:51 PM, Roland McGrath wrote:
> > Grepping for 'case 0:' shows exactly 8 places where this warning
> > should arise.  One of those is the __hurd_fail inline in hurd.h, so
> > that one instance probably generates the warning in many
> > compilations.

Correct.

> > Most of these are switches with only a few cases where it would be
> > easy to just change them to use 'if' instead of 'switch' without
> > complicating the code.  Doing that would just avoid the question.

That's another option.  (Which obvious detail am I missing though: why
doesn't GCC warn in this case (see below) if it does warn for »case 0:«
in switch?)

Another option is to cast the error_t to int in the switch statements
(but we typically want to avoid such casts).  There is no issue with user
programs doing »switch (errno)« and including a »case 0:«, because that
a) is not a valid thing to do as errno only is to be evaluated if it has
been set before (and that having been indicated by the failing function),
and b) errno at this level is not a error_t but a plain int.

> > I'm not real sure about giving an E* name to 0, since E* names are
> > for error codes, and 0 isn't one.  But OTOH for error_t 0 is
> > certainly one of the expected values, so it makes sense to have an
> > enum constant for it.  The question is what name to use, and then
> > whether to actually use that name or just add it to suppress the
> > warning while still using 0 in the code.
> > 
> > I tried GCC 4.4, and it doesn't give this warning when you use an
> > integer literal as long as the enum has some name for that integer
> > value.  If other GCC versions do warn for 'case 0:' when some item
> > 'foo = 0' is in the enum, someone please tell me.

$ for v in {4..7}; do echo gcc-4.$v && echo 'typedef enum __error_t_codes { 
EFOO = 1 } error_t; int foo (error_t err) { if (err == 0) return 0; return 1; 
}' | gcc-4.$v -Wall -c -x c - -o /dev/null; done
gcc-4.4
gcc-4.5
gcc-4.6
gcc-4.7

$ for v in {4..7}; do echo gcc-4.$v && echo 'typedef enum __error_t_codes { 
EFOO = 1 } error_t; int foo (error_t err) { switch (err) { case 0: return 0; 
default: break; } return 1; }' | gcc-4.$v -Wall -c -x c - -o /dev/null; done
gcc-4.4
gcc-4.5
: In function ‘foo’:
:1:91: warning: case value ‘0’ not in enumerated type ‘error_t’
gcc-4.6
: In function ‘foo’:
:1:91: warning: case value ‘0’ not in enumerated type ‘error_t’ 
[-Wswitch]
gcc-4.7
: In function ‘foo’:
:1:91: warning: case value ‘0’ not in enumerated type ‘error_t’ 
[-Wswitch]

$ for v in {4..7}; do echo gcc-4.$v && echo 'typedef enum __error_t_codes { 
ESUCCESS = 0, EFOO = 1 } error_t; int foo (error_t err) { if (err == 0) return 
0; return 1; }' | gcc-4.$v -Wall -c -x c - -o /dev/null; done
gcc-4.4
gcc-4.5
gcc-4.6
gcc-4.7

> > Assuming the warning behavior is consistent with what I've seen, my
> > inclination is not to change the code to use a symbolic name for
> > this.  That's mostly because the obvious choices like ESUCCESS or
> > EOK just seem wrong to me since the E* name space ought to be for
> > actual error codes, and I can't think of a good name I'd actually
> > want to be writing in the code.  So I'm inclined to pick some name
> > that is not very friendly to use (i.e. long and verbose), perhaps
> > even in the _* space, and add that to the error_t enum just for the
> > purpose of suppressing these warnings and not expect people to use
> > it in the code.

That could be done in sysdeps/mach/hurd/errnos.awk:BEGIN to avoid having
to go through the manual.


Grüße,
 Thomas


pgpzpmH8RK3Gt.pgp
Description: PGP signature

Re: Warnings about error_t in glibc on GNU/Hurd

[Barry deFreese]

How about ENOERR?? :)  Sorry, couldn't resist.

BTW, Thomas has a listing of several of the errors/warnings listed here:
http://www.gnu.org/software/hurd/open_issues/glibc.html#index1h1

I'm certainly fine with just adding a value for 0.

Thanks for taking the time to reply.

Barry

On 7/23/2012 3:51 PM, Roland McGrath wrote:
> Grepping for 'case 0:' shows exactly 8 places where this warning
> should arise.  One of those is the __hurd_fail inline in hurd.h, so
> that one instance probably generates the warning in many
> compilations.
> 
> Most of these are switches with only a few cases where it would be
> easy to just change them to use 'if' instead of 'switch' without
> complicating the code.  Doing that would just avoid the question.
> 
> I'm not real sure about giving an E* name to 0, since E* names are
> for error codes, and 0 isn't one.  But OTOH for error_t 0 is
> certainly one of the expected values, so it makes sense to have an
> enum constant for it.  The question is what name to use, and then
> whether to actually use that name or just add it to suppress the
> warning while still using 0 in the code.
> 
> I tried GCC 4.4, and it doesn't give this warning when you use an
> integer literal as long as the enum has some name for that integer
> value.  If other GCC versions do warn for 'case 0:' when some item
> 'foo = 0' is in the enum, someone please tell me.
> 
> Assuming the warning behavior is consistent with what I've seen, my
> inclination is not to change the code to use a symbolic name for
> this.  That's mostly because the obvious choices like ESUCCESS or
> EOK just seem wrong to me since the E* name space ought to be for
> actual error codes, and I can't think of a good name I'd actually
> want to be writing in the code.  So I'm inclined to pick some name
> that is not very friendly to use (i.e. long and verbose), perhaps
> even in the _* space, and add that to the error_t enum just for the
> purpose of suppressing these warnings and not expect people to use
> it in the code.
> 
> 
> Thanks,
> Roland
> 
> 

Re: Warnings about error_t in glibc on GNU/Hurd

[Roland McGrath]

Grepping for 'case 0:' shows exactly 8 places where this warning
should arise.  One of those is the __hurd_fail inline in hurd.h, so
that one instance probably generates the warning in many
compilations.

Most of these are switches with only a few cases where it would be
easy to just change them to use 'if' instead of 'switch' without
complicating the code.  Doing that would just avoid the question.

I'm not real sure about giving an E* name to 0, since E* names are
for error codes, and 0 isn't one.  But OTOH for error_t 0 is
certainly one of the expected values, so it makes sense to have an
enum constant for it.  The question is what name to use, and then
whether to actually use that name or just add it to suppress the
warning while still using 0 in the code.

I tried GCC 4.4, and it doesn't give this warning when you use an
integer literal as long as the enum has some name for that integer
value.  If other GCC versions do warn for 'case 0:' when some item
'foo = 0' is in the enum, someone please tell me.

Assuming the warning behavior is consistent with what I've seen, my
inclination is not to change the code to use a symbolic name for
this.  That's mostly because the obvious choices like ESUCCESS or
EOK just seem wrong to me since the E* name space ought to be for
actual error codes, and I can't think of a good name I'd actually
want to be writing in the code.  So I'm inclined to pick some name
that is not very friendly to use (i.e. long and verbose), perhaps
even in the _* space, and add that to the error_t enum just for the
purpose of suppressing these warnings and not expect people to use
it in the code.


Thanks,
Roland