On Fri, 19 May 2006, Marcel Siegert wrote:
> 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)
That is a good reason for having padding. I'm not saying, get rid of all the
padding. Some padding should be kept for just those reasons. But padding
inside each struct, and in the union those structs are in, isn't necessary.
Look at all the dvbfe_xxx_info structs. They all have 32 bytes of padding.
This means they are all bigger than 32 bytes, right? Then the union they are
in has 32 bytes of padding. But all the structs are bigger than 32 bytes, so
this padding does nothing!
> > 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
Yes, I know that. But if you look at that definition above, it starts
numbering with the most significant bits, so one of the enums has the value
1<<31. This way an extra dummy value isn't needed.
> 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.
You could also say that allocating bits starting at the most significant is
the natural order too. But why does it matter? No one needs to look at the
values, so who is going to be confused? Assuming that the sequence 31, 30,
29, 28, 27, 26 is going to confuse anyone. Do you really think anyone could
look at that sequence and not figure out what number comes next?
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
