[PATCH 8/8] firmware: mark some structs as packed
laforge at gnumonks.org
Thu Nov 24 10:33:09 UTC 2011
On Thu, Nov 24, 2011 at 10:36:27AM +0100, Alexander Huemer wrote:
> On Thu, Nov 24, 2011 at 07:58:33AM +0100, Sylvain Munaut wrote:
> > Hi,
> > First, thanks for this series of patch, I'll be sure to review and test them.
> Well, no problem, I had some time on hand I couldn't use otherwise.
> > > this eliminates the occurrance of gcc warning
> > > warning: cast increases required alignment of target type
> > > ---
> > > src/target/firmware/include/comm/timer.h | 4 +++-
> > > src/target/firmware/include/layer1/sched_gsmtime.h | 5 ++++-
> > > 2 files changed, 7 insertions(+), 2 deletions(-)
> > > -};
> > > +} __packed;
> > I'm not conviced by this one : Why should we use packed for structures
> > not used as 'communication packets' ?
> What are the drawbacks ?
normally, the compiler will lay out the structure in a way that
optimizes accesses to members. So e.g. on an ARM, a
will very likely contain 3 bytes padding between the two uint8 values in
order to make sure no unaligned loads/stores will be required.
However, if you change that to '__packed', the padding will not be
generated and any access to struct members will need to deal with
unaligned accesses, which can be inflating the code size considerably.
> Steve and Holger raised some reasonable concerns on some of the other
> patches, it seems I was quite unconcentrated and they are mostly crap.
don't say that. We love to receive cleanup patches like yours, and
even if 2-3 have some issues, at a series of 8 it is still considerable
gain for the project.
> The patches do not contain any "magic" anyway. Just straight-forward
> reaction on the different kind of warnings. Most of them can be
> eliminated easily.
yes, in most cases. However, sometimes we keep the warning around as a
reminder that something still needs to be done, such as e.g. handling an
undhandled enum value in switch() or the like. In such a case of course
we shuold implement the missing member, and not introduce a default
- Harald Welte <laforge at gnumonks.org> http://laforge.gnumonks.org/
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
More information about the baseband-devel