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. :(
-Peff
