Jeff King <peff@xxxxxxxx> writes: > On Mon, Nov 04, 2024 at 12:41:38PM +0100, Karthik Nayak wrote: > >> Range-diff against v4: >> 1: b3d518e998 = 1: 6c00e25c86 packfile: add repository to struct `packed_git` >> 2: bb9d9aa744 = 2: 70fc8a79af packfile: use `repository` from `packed_git` directly >> 3: d5df50fa36 = 3: 167a1f3a11 packfile: pass `repository` to static function in the file >> 4: 0107801c3b = 4: b7cfe78217 packfile: pass down repository to `odb_pack_name` >> 5: 2d7608a367 = 5: 5566f5554c packfile: pass down repository to `has_object[_kept]_pack` >> 6: 2c84026d02 = 6: 1b26e45a9b packfile: pass down repository to `for_each_packed_object` >> 7: 84b89c8a0e ! 7: 7654bf5e7e config: make `delta_base_cache_limit` a non-global variable >> @@ Commit message >> this value from the repository config, since the value is only used once >> in the entire subsystem. >> >> + The type of the value is changed from `size_t` to an `unsigned long` >> + since the default value is small enough to fit inside the latter on all >> + platforms. >> + > > I think this change is not ideal, for the same reason that the other > type changes were: you can conceivably have a 4GB or larger cache here. > On Windows using "unsigned long" would prevent that. (On most other > systems it is OK either way since "unsigned long" and "size_t" are > generally the same size). > > I do think the config parsing should change to use size_t here (like I > mentioned elsewhere in the thread), which would fix it on Windows. > That's outside the scope of your patch, but in the meantime we should > not be making things worse by moving the variable itself to the inferior > type. > There is a subtle difference though. Both the configs (this and next commit) although are initialized with 'size_t' they are bounded by 'unsigned long's range, since they do to the functions used to obtain the information. The only difference being the defaults, which could've been greater than the range of 'unsigned long'. While having said that, keeping it 'size_t' makes it easier to know which configs need to use the yet_to_be introduced 'size_t' variants of the config parsers. So let me change this too. Thanks. Karthik
Attachment:
signature.asc
Description: PGP signature
