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. -Peff
