[Freeciv-Dev] (PR#15840) common/dataio.c:700 is dubious

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=15840 >

> [guillaume.melquiond@xxxxxxxxx - Fri Mar 10 20:30:39 2006]:
> 
> I was giving a shot at compiling freeciv (gtk client) with gcc 4.1 and
> it went surprisingly well. The only type-prunning it detected was at
> common/dataio.c:702. A pointer to an enumeration (possibly a single
> char) is transformed into a pointer to int, and then used to store an
> int in memory. Actually, this does not crash since this is the first
> field of the structure and hence it is aligned; and moreover the other
> fields it overrides along the way do not matter.
> 
> Still this is really bad practice and I was about to fix it. But then
> I noticed that only four of the six fields of player_diplstate are
> serialized: max_state and has_reason_to_cancel are neither serialized
> nor initialized. So I'm wondering: Do these serialization functions
> really do what they are expected to do?

The dataio code is pretty old and has not been closely inspected.  There
was an error of this type in packets_gen.c that caused garbage data to
be sent on (I forget exactly) reverse-endian or 64-bit systems.  It
seems likely this code would also fail in those situations, and must be
fixed.

As for why those other two fields aren't serialized, it is most likely
because they were added to the diplstate field after the dataio code was
written, and because they're not used anywhere in the client there was
no reason to send them.  Of course there's no reason not to send them
either, and in the long run it's better to have proper serialization.

Anyway, a patch to fix this would be welcome.

-jason