On 20/10/2019 00:23, Jeff King wrote:
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
declarations, 2019-04-05)
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).
Thanks, I'll add that one to my list of size values that I should check
when rebasing my current series.
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)
Philip
