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

[Segher Boessenkool]

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