Re: [Xen-devel] [PATCH 3 of 6] x86/mm: Another couple of comparisons of unsigned vars with < 0

[Tim Deegan <tim@xxxxxxx>]

At 15:19 +0100 on 05 Apr (1333639188), Jan Beulich wrote:
> >>> On 05.04.12 at 16:08, Tim Deegan <tim@xxxxxxx> wrote:
> > At 14:31 +0100 on 05 Apr (1333636297), Jan Beulich wrote:
> >> >>> On 05.04.12 at 14:07, Tim Deegan <tim@xxxxxxx> wrote:
> >> > x86/mm: Another couple of comparisons of unsigned vars with < 0.
> >> > 
> >> > Adding the explicit (unsigned) casts in case enums ever end up signed.
> >> 
> >> Ugly, but unavoidable if range comparisons are done on enums (which
> >> is what ought to get fixed instead imo).
> > 
> > Any suggestions for how?  Turn the array access into a case statement?
> > That's even uglier IMO. 
> 
>       switch (<enum-var>) {
>       case <low> ... <high>:
>               break;
>       default:
>               <bad>;
>       }

Hmm.  I really prefer the cast to that.

In both these cases, I could replace this idiom:

 static const array a  = { val_x, val_y, val_z };
 if ( (unsigned) e >= enum_z ) return -EINVAL;
 val = a[e];

with 

 switch (e) {
     case enum_x: val = val_x; break;
     case enum_x: val = val_x; break;
     case enum_x: val = val_x; break;
     default: return -EINVAL;
 }

but unless it's laid out like this (one line per case) it'll get ugly too.

Switching to #defines is a bit tricky as they're in a public header and
I'm not confident we wouldn't break someone else's compile.

> doesn't look that bad. But when wanting to do range comparisons,
> an enum may not be the best choice anyway (for the very reason of
> it possible being signed, and signed array indexes generally being
> less efficient than unsigned ones), and hence using #define-s and
> an "unsigned int" variable might be the better way.
> 
> Otoh, doesn't clang have a command line option to control whether
> enums are signed or unsigned? I think gcc does.

The only gcc flag I can find is -fshort-enums, for making enums into
shorter types.

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel