Re: [PATCH/RFC v1 1/1] Use size_t instead of unsigned long
- Date: Sun, 18 Nov 2018 15:18:52 -0500
- From: Derrick Stolee <stolee@xxxxxxxxx>
- Subject: Re: [PATCH/RFC v1 1/1] Use size_t instead of unsigned long
On 11/17/2018 10:11 AM, tboegi@xxxxxx wrote:
From: Torsten Bögershausen <tboegi@xxxxxx>
Currently Git users can not commit files >4Gib under 64 bit Windows,
where "long" is 32 bit but size_t is 64 bit.
Improve the code base in small steps, as small as possible.
What started with a small patch to replace "unsigned long" with size_t
in one file (convert.c) ended up with a change in many files.
Signed-off-by: Torsten Bögershausen <tboegi@xxxxxx>
This needs to go on top of pu, to cover all the good stuff
Better to work on top of 'master', as the work in 'pu' will be rewritten
several times, probably.
I have started this series on November 1st, since that 2 or 3 rebases
had been done to catch up, and now it is on pu from November 15.
I couldn't find a reason why changing "unsigned ling"
into "size_t" may break anything, any thoughts, please ?
IIRC, the blocker for why we haven't done this already is that "size_t",
"timestamp_t" and "off_t" are all 64-bit types that give more implied
meaning, so we should swap types specifically to these as we want. One
example series does a specific, small change .
If we wanted to do a single swap that removes 'unsigned long' with an
unambiguously 64-bit type, I would recommend "uint64_t". Later
replacements to size_t, off_t, and timestamp_t could happen on a
case-by-case basis for type clarity.
It may benefit reviewers to split this change into multiple patches,
starting at the lowest levels of the call stack (so higher 'unsigned
long's can up-cast to the 64-bit types when calling a function) and
focusing the changes to one or two files at a time (X.c and X.h,
Since you are talking about the benefits for Git for Windows to accept
4GB files, I wonder if we can add a test that verifies such a file will
work. If you have such a test, then I could help verify that the test
fails before the change and succeeds afterward.
Finally, it may be good to add a coccinelle script that replaces
'unsigned long' with 'uint64_t' so we can easily fix any new
introductions that happen in the future.
Thanks! I do think we should make this change, but we must be careful.
It may be disruptive to topics in flight.