Re: [PATCH] dir: support platforms that require aligned reads

[Jeff King <peff@peff.net>]

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