Re: [PATCH 5/6] Allow to use crc32 as a lighter checksum on index

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

 



2012/2/5 Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>:
>        if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
>                return error("bad signature");
> -       if (hdr->hdr_version != htonl(2) &&
> -           hdr->hdr_version != htonl(3) &&
> -           hdr->hdr_version != htonl(4))
> +       if (hdr->hdr_version == htonl(2) ||
> +           hdr->hdr_version == htonl(3))
> +               do_crc = 0;
> +       else if (hdr->hdr_version == htonl(4)) {
> +               struct ext_cache_header *ehdr = (struct ext_cache_header *)hdr;
> +               do_crc = ntohl(ehdr->hdr_flags) & CACHE_F_CRC;
> +       }
> +       else

Ick. Ick. Ick. Please $DEITY no.

When it comes to data integrity codes in Git... PICK ONE AND STICK WITH IT.

If CRC-32 is good enough to protect the index content such that disk
corruption is probably detectable with it, lets just switch to CRC-32
in index version 4. Don't make it optional with a new header field
that wasn't there in version 3 and is now only able to accept 32 bits
of flags before we have to go and create index version 5. We already
have a cache extension system available with extension codes in the
footer of the index file. We don't need YET ANOTHER EXTENSION SYSTEM.

If CRC-32 is not good enough, and we don't want to trust it (or
really, YOU don't want to trust it) please do not then go and propose
that a less knowledgeable user should switch to CRC-32 "because it is
faster". If we don't want to rely on the error detection of CRC-32,
then we should be using SHA-1. Or SHA-256.


I haven't really put a lot of thought into this. But I suspect CRC-32
is sufficient on the index file, until it gets so big that the
probability of a bit flip going undetected is too high due to the size
of the file, but then we are into the "huge" index size range that has
you trying to swap out SHA-1 for CRC-32 because SHA-1 is too slow. Uhm
no.

CRC-32 may be good enough, we use it inside of the pack-objects when
doing repacking locally and don't want to inflate objects to check
SHA-1, but do want to try and detect a random bit flip caused by a
broken file copier. Thus far its held up well there. Given the very
transient nature of the index file (and how it can be mostly rebuilt
from a tree object and the working directory), CRC-32 might be good
enough. But please pick one.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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]