Re: [PATCH v3 3/9] finalize_object_file(): implement collision check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 06, 2024 at 03:46:12PM -0400, Taylor Blau wrote:
> diff --git a/object-file.c b/object-file.c
> index 54a82a5f7a0..85f91516429 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1899,6 +1899,57 @@ static void write_object_file_prepare_literally(const struct git_hash_algo *algo
>  	hash_object_body(algo, &c, buf, len, oid, hdr, hdrlen);
>  }
>  
> +static int check_collision(const char *filename_a, const char *filename_b)
> +{
> +	char buf_a[4096], buf_b[4096];
> +	int fd_a = -1, fd_b = -1;
> +	int ret = 0;
> +
> +	fd_a = open(filename_a, O_RDONLY);
> +	if (fd_a < 0) {
> +		ret = error_errno(_("unable to open %s"), filename_a);
> +		goto out;
> +	}
> +
> +	fd_b = open(filename_b, O_RDONLY);
> +	if (fd_b < 0) {
> +		ret = error_errno(_("unable to open %s"), filename_b);
> +		goto out;
> +	}
> +
> +	while (1) {
> +		ssize_t sz_a, sz_b;
> +
> +		sz_a = read_in_full(fd_a, buf_a, sizeof(buf_a));
> +		if (sz_a < 0) {
> +			ret = error_errno(_("unable to read %s"), filename_a);
> +			goto out;
> +		}
> +
> +		sz_b = read_in_full(fd_b, buf_b, sizeof(buf_b));
> +		if (sz_b < 0) {
> +			ret = error_errno(_("unable to read %s"), filename_b);
> +			goto out;
> +		}
> +
> +		if (sz_a != sz_b || memcmp(buf_a, buf_b, sz_a)) {
> +			ret = error(_("files '%s' and '%s' differ in contents"),
> +				    filename_a, filename_b);
> +			goto out;
> +		}
> +
> +		if (sz_a < sizeof(buf_a))
> +			break;
> +	}
> +
> +out:
> +	if (fd_a > -1)
> +		close(fd_a);
> +	if (fd_b > -1)
> +		close(fd_b);
> +	return ret;
> +}
> +
>  /*
>   * Move the just written object into its final resting place.
>   */

This function compares the exact contents, but isn't that wrong? The
contents may differ even though the object is the same because the
object hash is derived from the uncompressed data, whereas we store
compressed data on disk.

So this might trigger when you have different zlib versions, but also if
you configure core.compression differently.

Patrick




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux