hi
Trent Piepho wrote:
On Fri, 19 May 2006, Manu Abraham wrote:
Trent Piepho wrote:
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]
I think maybe that isn't what he meant. Maybe Johannes can provide
reasoning why structures in a union and the union itself all need padding.
the might be 2 reasons:
a) don't get any kind of e.g. memcopy problems with new structs and old kernels after
this patch went in. so the size of e.g. the tuning params for dvb-s2 can be extended
without having to change the structs size. so also old kernel will perform a correct
memcopy.
b) the union as a member of the params struct is a large as its biggest member.
so we just reserve some space, because we do not know actually, how e.g. DVB-C2/T2 or
something similar will use as its struct size. as size of the params is used to
calculate the ioctl itself, just adding a new struct that has a size 1 byte more than
the actual reserved would cause backwards incompatibility in future. (this is the
actual trapdoor we felt into and that we want to prevent off in the future)
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
If you do that, you will lose the ability of gcc to warn you if you forget
to handle a case. I do not think the API should force application
programmers to use a certain style. Obviously, someone likes this warning
or gcc wouldn't have it.
to use default i a correct way, i wouldn't just return on the default:, but print out an error message, that an
unrecognised value was received. that is the correct behaviour, because you may have
differences on userspace apps and kernel. and those aren't detectable at compiletime.
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 .. ?
Like this:
enum dvbfe_interleaver {
DVBFE_INTERLEAVER_IGNORE = (0 << 0),
DVBFE_INTERLEAVER_NATIVE = (1 << 31),
DVBFE_INTERLEAVER_INDEPTH = (1 << 30),
DVBFE_INTERLEAVER_AUTO = (1 << 29)
};
Now the DUMMY value isn't needed. One less symbol is defined, and there is
no issuse with warnings on switch statements. No one needs to look at the
value of DVBFE_INTERLEAVER_NATIVE, so who will care that it is now (1<<31)
instead of (1<<0)?
the dummy value is needed to keep struct sizes on different architectures. e.g. arm
the << 31 guarantees that an unsigned int 32 bit wide will be used as the underlaying
internal integer representation type.
my 0.02$:
and as, from my knowledge, counting starts at 0 in a computer environment/programming language
normally. so your idea is practicable to avoid a value, but non standard in normal coding
logic. that even may confuse people more than having a specialised dummy value.
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
