Trent Piepho wrote:
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.
Well, it was a suggestion from Johannes. pad[32] and pad [128]
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
use default. That will not give you any warnings
switch (d) {
case DVBFE_DELSYS_DVBS:
break;
case DVBFE_DELSYS_ATSC:
break;
default:
break;
}
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.
DUMMY is indeed dummy, not for any practical usage.
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 didn't really understand what you meant. You mean (1 << 31) should
have been the first field ?
But i don't see any pronlem in using the current way, other than a
cosmetic issue, but even in a cosmetic case, we start from 0 and go up
normally, don't we .. ?
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.
I too liked that way, so did some others too, but Johannes did not like
it, as i understood from his comments.
Anyway it is not a change that does hurt in anyway, i don't mind either
way, since we have been discussing about this since November last year.
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
