On 20/10/2019 00:23, Jeff King wrote:
Thanks, I'll add that one to my list of size values that I should check
when rebasing my current series.
On Sat, Oct 19, 2019 at 09:20:11PM +0200, Christian Couder wrote:
+static void write_reused_pack_one(size_t pos, struct hashfile *out,
+ struct pack_window **w_curs)
+ off_t offset, next, cur;
+ enum object_type type;
+ unsigned long size;
Is this a mem_sized size or a counter for less that 4GiB items?
What I can see is that `&size` is passed as the last argument to
unpack_object_header() below. And unpack_object_header() is defined in
packfile.h like this:
int unpack_object_header(struct packed_git *, struct pack_window **,
off_t *, unsigned long *);
since at least 336226c259 (packfile.h: drop extern from function
So fixing this, if it needs to be fixed, should probably be part of a
separate topic fixing unpack_object_header().
Yeah, this one definitely should be moved to whatever we used to
represent object sizes in the future (size_t, or I guess off_t if we
really want to handle huge objects on 32-bit systems too). But
definitely it shouldn't happen in this series, and I don't think anybody
interested in the other topic (converting the integer type for object
sizes) needs to keep tabs on it. When they convert
unpack_object_header(), the compiler will complain because of passing
it as a pointer (the more insidious ones will be where we return an
unsigned long to represent an object type, and somebody will have to
look into every caller).
If I understand correctly gcc is no longer detecting those size
conversions, but clang has -Wshorten-64-to-32 but I've still to check if
it catches some of these conversion issues (esp when int (32) is
extended to 64 bit size_t, rather than being 64 bit in the first place)