On Thu, 18 May 2006, Manu Abraham wrote:
> Manu Abraham wrote:
> > Hi All,
> >
> > Updated the patch again
>
>
> Found 5 additions/fixes. Any comments other than these ?
Why is there padding in both the xxxx_params structs, and in the union that
contains them? Same for the xxxx_info structs and the union they are in. If
you have the padding value in the union, the padding in each of the structs in
the union isn't necessary.
If you write a switch statement like this:
switch(d) {
case DVBFE_DELSYS_DVBS: return 1;
case DVBFE_DELSYS_DSS: return 1;
case DVBFE_DELSYS_DVBS2: return 1;
case DVBFE_DELSYS_DVBC: return 1;
case DVBFE_DELSYS_DVBT: return 1;
case DVBFE_DELSYS_DVBH: return 1;
case DVBFE_DELSYS_ATSC: return 1;
}
You will get a warning message from gcc:
foo.c:14: warning: enumeration value 'DVBFE_DELSYS_DUMMY' not handled in switch
gcc has the ability to warn you if a switch on an enum doesn't handle every
value for the enum. This can be nice, as it will catch when the programmer
forgets about one of the enum's values. The only way to keep this check and
have no warning, is to put in a case for the _DUMMY value. This is bad, DUMMY
is just there to keep the enums the right size on some architechtures with
short enums. No one should ever use it.
This problem can be avoided, if the enums start at (1<<31) and go down instead
of (1<<0) and go up. Then all the DUMMY values can be eliminated. The values
are just arbitrary numbers, there shouldn't be any problem in going the order
direction.
I liked it better when the capabilities ioctl used the same structs as the
set/get parameters ioctls. It had a nice symmetry and there were fewer
data types to keep track of.
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
