On Wed, Oct 25, 2023 at 09:58:06AM +0200, Patrick Steinhardt wrote:

On Mon, Oct 23, 2023 at 06:45:01PM -0400, Taylor Blau wrote:

quoted

Introduce `index_blob_bulk_checkin_incore()` which allows streaming
arbitrary blob contents from memory into the bulk-checkin pack.

In order to support streaming from a location in memory, we must
implement a new kind of bulk_checkin_source that does just that. These
implementation in spread out across:

Nit: the commit message is a bit off here. Probably not worth a reroll
though.

Your eyes are definitely mine, because I'm not seeing where the commit
message is off! But hopefully since you already don't think it's worth a
reroll, and I'm not even sure what the issue is that we can just leave
it ;-).

quoted

+static off_t bulk_checkin_source_seek_incore(struct bulk_checkin_source *source,
+					     off_t offset)
+{
+	if (!(0 <= offset && offset < source->size))
+		return (off_t)-1;

At the risk of showing my own ignorance, but why is the cast here
necessary?

It's not necessary, see this godbolt example that shows two functions
doing the same thing (one with the explicit cast, one without):

  https://godbolt.org/z/f737EcGfG

But there are a couple of other spots within the bulk-checkin code
(specifically within the deflate_blob_to_pack() routine) that explicitly
cast -1 to an off_t, so I was more trying to imitate the local style.

Thanks,
Taylor