Re: CVS commit: src/sys/arch/atari/stand/installboot

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

>    -	uint16_t	 sum;
>    +	union {
>    +		struct bootblock *bbp;
>    +		uint16_t *word;		/* to fill cksum word */
>    +	} bbsec;
>    ...
>    -	sum = 0;
>    -	memcpy(bb->bb_xxboot + 255 * sizeof(sum), &sum, sizeof(sum));
>    -	sum = 0x1234 - abcksum(bb->bb_xxboot);
>    -	memcpy(bb->bb_xxboot + 255 * sizeof(sum), &sum, sizeof(sum));
>    +	bbsec.bbp = bb;
>    +	bbsec.word[255] = 0;
>    +	bbsec.word[255] = 0x1234 - abcksum(bb->bb_xxboot);
> 
> Um, that has the same issue as the original code, no? It still refers
> to the content of the struct bootblock object by two different types,
> struct bootblock and uint16_t -- passing the pointer through a union
> doesn't change that.

IIUC, union is valid in this case per C9899:201x 6.5 Expressions:
---
7 An object shall have its stored value accessed only by
  an lvalue expression that has one of the following types:

 :
 * an aggregate or union type that includes one of the aforementioned
   types among its members (including, recursively, a member of a
   subaggregate or contained union),
---
My understanding is the strict aliasing rule is referred by compiler
for reorder optimization etc.  If the access via union is still invalid,
why does the strict gcc48 no longer complain against it?

> What's wrong with the memcpy?

Did you read whole this thread?

See
http://mail-index.netbsd.org/source-changes-d/2014/11/14/msg007328.html
>> - The MD installboot implementation heavily depends on hardware specific
>>   features but there are few documents and descriptions about such hardware
>>   except existing source code, so it's much more important to keep
>>   intention of the original author in such MD code for future readers
>>   than appeasing strict modern compilers by mechanical changes.

It's TierII MD code, so maintainability is much more important than
perfection (unless you will take maintainership of this installboot).

> I don't see any byte ordering issues here that weren't present before.

Currently it may work.  But once we will try to make the MD installboot
merged into MI usr.sbin/installboot, accessing variables via different
types always confuse programmers who don't know actuall intention
(which is the host endian value, which is the target endian value etc).

---
Izumi Tsutsui