[SLOF] [PATCH 1/4] romfs/tools: Remove superfluous union around the rom header struct

Segher Boessenkool segher at kernel.crashing.org
Fri Jul 20 02:43:45 AEST 2018


On Thu, Jul 19, 2018 at 07:34:24PM +1000, Alexey Kardashevskiy wrote:
> On 19/7/18 12:35 am, Segher Boessenkool wrote:
> > On Wed, Jul 18, 2018 at 02:41:41PM +0200, Thomas Huth wrote:
> >> -	union {
> >> -		unsigned char pcArray[FLASHFS_HEADER_DATA_SIZE];
> >> -		struct stH stHeader;
> >> -	} uHeader;
> >> +	struct stH stHeader;
> >> +
> >> +	assert(sizeof(stHeader) == FLASHFS_HEADER_DATA_SIZE);
> >>  
> >>  	/* initialize Header */
> >> -	memset(uHeader.pcArray, 0x00, FLASHFS_HEADER_DATA_SIZE);
> >> +	memset(&stHeader, 0x00, FLASHFS_HEADER_DATA_SIZE);
> > 
> > It would be nice to have a build-time assert for this.
> > 
> > You can also get rid of this explicit memset by doing this as
> > 
> > 	assert(sizeof(stHeader) == FLASHFS_HEADER_DATA_SIZE);
> > 
> > 	struct stH stHeader = { 0 } ;
> 
> 
> I do not really know where this flashfs/romfs format came from so I'll
> simply ask why precisely we want this assert? Increasing
> FLASHFS_HEADER_DATA_SIZE to 0x88 does not make any difference as the
> header size is stored in a file header (is that even correct thing to say?)?

The code is treating some binary data as a C struct.  Not a good idea at all,
but yes it helps to verify the sizes!  And the offsets, and the endianness,
and that there is no padding anywhere, etc.


Segher


More information about the SLOF mailing list