Am 16.07.2017 um 16:04 schrieb Jeff King:
> On Sun, Jul 16, 2017 at 02:17:37PM +0200, René Scharfe wrote:
>
>> -static void stat_data_from_disk(struct stat_data *to, const struct
>> stat_data *from)
>> +static void stat_data_from_disk(struct stat_data *to, const unsigned char
>> *data)
>> {
>> - to->sd_ctime.sec = get_be32(&from->sd_ctime.sec);
>> - to->sd_ctime.nsec = get_be32(&from->sd_ctime.nsec);
>> - to->sd_mtime.sec = get_be32(&from->sd_mtime.sec);
>> - to->sd_mtime.nsec = get_be32(&from->sd_mtime.nsec);
>> - to->sd_dev = get_be32(&from->sd_dev);
>> - to->sd_ino = get_be32(&from->sd_ino);
>> - to->sd_uid = get_be32(&from->sd_uid);
>> - to->sd_gid = get_be32(&from->sd_gid);
>> - to->sd_size = get_be32(&from->sd_size);
>> + memcpy(to, data, sizeof(*to));
>> + to->sd_ctime.sec = ntohl(to->sd_ctime.sec);
>> + to->sd_ctime.nsec = ntohl(to->sd_ctime.nsec);
>> + to->sd_mtime.sec = ntohl(to->sd_mtime.sec);
>> + to->sd_mtime.nsec = ntohl(to->sd_mtime.nsec);
>> + to->sd_dev = ntohl(to->sd_dev);
>> + to->sd_ino = ntohl(to->sd_ino);
>> + to->sd_uid = ntohl(to->sd_uid);
>> + to->sd_gid = ntohl(to->sd_gid);
>> + to->sd_size = ntohl(to->sd_size);
>> }
>
> Hmm. I would have written this to pull the bytes directly out of the
> array, like:
>
> to->sd_ctime.sec = get_be32(data); data += 4;
> to->sd_ctime.nsec = get_be32(data); data += 4;
>
> etc. Or even a helper to do the advancing like:
>
> to->sd_ctime.sec = parse_be32(&data);
>
> That reduces assumptions about padding in "struct stat_data". But
> looking more at this code, and reading your comment:
>
>> Side note: The OS name is not enough for determining the layout of
>> struct ondisk_untracked_cache. Different platforms can have different
>> int sizes and padding. Adding the machine type could help, but that
>> would be a breaking change. At that point we would be better off
>> defining a machine-independent format, no?
>
> it looks like assumptions about struct layout are pervasive and part of
> the on-disk format. Yuck. :(
Assuming that there is no padding probably even works for the platforms
the code currently supports (basically x86), but I don't know about
others. We'd need to change the writing side as well to match, though.
Which is probably a good idea, but I tried to keep the patch small and
its impact low. Cross-machine usability is currently explicitly not
supported -- not sure why, though.
René
