Re: Index format v5

> I think it is possible to remove the last magic number and also to
> make the CRC handling easier.  I have pushed some suggested changes
> to github [1]:
> 
> 1. With the current code, trying to read a file that is less than 24
> bytes long would result in a struct.error (because it would try to
> unpack a string that is shorter than the struct) whereas the
> underlying error in this case should almost always be reported as a
> signature error.  So it is correct to read the signature separately
> from the rest of the header, but it is even more correct to check
> the signature (including its length) before reading on.

> 2. I introduce a class CRC to hold checksums, so that (a) the code
> for handling checksums can be encapsulated, and (b) an instance of
> this class can be passed into functions and mutated in-place, which
> is less cumbersome than requiring functions to return (data, crc)
> tuples.  This, in turn, makes possible...
> 
> 3. ...a new function read_struct(f, s, crc), which reads the data
> for a struct.Struct from f, checksums it, and returns the unpacked
> data.  This function is more convenient to use than the old
> read_calc_crc().
> 
> 4. The checksum instance can also be made responsible for verifying
> that the next four bytes in the file agree with the expected
> checksum.  This removes some more code duplication.  (See
> CRC.matches().)
>
> 5. You read the extension offsets using CRC_STRUCT.size, which is
> technically correct but misleading.  In fact, the extension offsets
> should be documented using their own EXTENSION_INDEX_STRUCT.  Also,
> it makes more sense to store the unpacked integer offsets to
> extoffsets rather than the raw 4-byte strings.
> 
> 6. With a couple of more minor changes it is possible to replace the
> magic number "24" in read_index_entries().  With this change the
> computation is documented very explicitly and is also (somewhat)
> robust against future changes in the format.
> 
> Look over my changes and take whatever you want.