Re: [PATCH 02/17] chunk-format: create chunk format write API

On 1/26/2021 9:42 PM, Taylor Blau wrote:

On Tue, Jan 26, 2021 at 04:01:11PM +0000, Derrick Stolee via GitGitGadget wrote:

quoted

+/*
+ * When writing a chunk-based file format, collect the chunks in
+ * an array of chunk_info structs. The size stores the _expected_
+ * amount of data that will be written by write_fn.
+ */
+struct chunk_info {
+	uint32_t id;
+	uint64_t size;

Hmm. Would we not want an off_t to indicate the size here?

I wondered briefly if we even needed a size field at all, since calling
write_fn would tell us the number of bytes written. But I suppose you
want to know ahead of time so that you can write the file in one pass
(beginning with the table of contents, which certainly needs to know the
size).

Is off_t 64-bits on a 32-bit machine? This is intentionally typed
to be "64 bits no matter what" because it correlates with the file
format's size for the chunk offsets.

quoted

+		if (cf->f->total + cf->f->offset != start_offset + cf->chunks[i].size)

I don't think this is a practical concern, but a malicious caller could
overflow this by passing a bogus "size" parameter. Maybe:

    uint64_t end_offset = ...;

    if (end_offset - start_offset != cf->chunks[i].size)
      BUG(...)

Sure.

Thanks,
-Stolee