git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Derrick Stolee <stolee@gmail.com>
To: tboegi@web.de, git@vger.kernel.org
Subject: Re: [PATCH/RFC v1 1/1] Use size_t instead of unsigned long
Date: Sun, 18 Nov 2018 15:18:52 -0500	[thread overview]
Message-ID: <c09938cf-7631-ef89-d8fc-d952f9b121c8@gmail.com> (raw)
In-Reply-To: <20181117151139.22994-1-tboegi@web.de>

On 11/17/2018 10:11 AM, tboegi@web.de wrote:
> From: Torsten Bögershausen <tboegi@web.de>
>
> 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@web.de>
> ---
>
> This needs to go on top of pu, to cover all the good stuff
>    cooking here.

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 [1].

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, 
preferrably).

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.

-Stolee

[1] https://public-inbox.org/git/20181112084031.11769-1-carenas@gmail.com/


  reply	other threads:[~2018-11-18 20:18 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-17 15:11 [PATCH/RFC v1 1/1] Use size_t instead of unsigned long tboegi
2018-11-18 20:18 ` Derrick Stolee [this message]
2018-11-18 23:40   ` Junio C Hamano
2018-11-19  5:33     ` Torsten Bögershausen
2018-11-19 18:15       ` René Scharfe
2018-11-19 16:33   ` Torsten Bögershausen
2018-11-20  1:36     ` Junio C Hamano
2018-11-20  5:04 ` [PATCH v2 1/1] Use size_t instead of 'unsigned long' for data in memory tboegi
2018-11-21 11:55   ` Derrick Stolee
2019-01-16 21:46   ` Thomas Braun
2019-01-19 17:06     ` Torsten Bögershausen
2019-01-22 14:25       ` Thomas Braun
2019-04-13 15:18 ` [PATCH v3 " tboegi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c09938cf-7631-ef89-d8fc-d952f9b121c8@gmail.com \
    --to=stolee@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).