git/mem-pool.c, branch v2.46.2

don't report vsnprintf(3) error as bug

2024-04-21T19:27:07Z

strbuf_addf() has been reporting a negative return value of vsnprintf(3) as a bug since f141bd804d (Handle broken vsnprintf implementations in strbuf, 2007-11-13). Other functions copied that behavior: 7b03c89ebd (add xsnprintf helper function, 2015-09-24) 5ef264dbdb (strbuf.c: add `strbuf_insertf()` and `strbuf_vinsertf()`, 2019-02-25) 8d25663d70 (mem-pool: add mem_pool_strfmt(), 2024-02-25) However, vsnprintf(3) can legitimately return a negative value if the formatted output would be longer than INT_MAX. Stop accusing it of being broken and just report the fact that formatting failed. Suggested-by: Jeff King Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

mem-pool: use st_add() in mem_pool_strvfmt()

2024-03-31T23:00:36Z

If len is INT_MAX in mem_pool_strvfmt(), then len + 1 overflows. Casting it to size_t would prevent that. Use st_add() to go a step further and make the addition *obviously* safe. The compiler can optimize the check away on platforms where SIZE_MAX > INT_MAX, i.e. basically everywhere. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

mem-pool: add mem_pool_strfmt()

2024-02-26T17:35:40Z

Add a function for building a string, printf style, using a memory pool. It uses the free space in the current block in the first attempt. If that suffices then the result can already be used without copying or reformatting. For strings that are significantly shorter on average than the block size (ca. 1 MiB by default) this is the case most of the time, leading to a better perfomance than a solution that doesn't access mem-pool internals. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

mem-pool: simplify alignment calculation

2023-12-28T20:22:58Z

Use DIV_ROUND_UP in mem_pool_alloc() to round the allocation length to the next multiple of GIT_MAX_ALIGNMENT instead of twiddling bits explicitly. This is shorter and clearer, to the point that we no longer need the comment that explains what's being calculated. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

mem-pool: fix big allocations

2023-12-28T20:22:43Z

Memory pool allocations that require a new block and would fill at least half of it are handled specially. Before 158dfeff3d (mem-pool: add life cycle management functions, 2018-07-02) they used to be allocated outside of the pool. This patch made mem_pool_alloc() create a bespoke block instead, to allow releasing it when the pool gets discarded. Unfortunately mem_pool_alloc() returns a pointer to the start of such a bespoke block, i.e. to the struct mp_block at its top. When the caller writes to it, the management information gets corrupted. This affects mem_pool_discard() and -- if there are no other blocks in the pool -- also mem_pool_alloc(). Return the payload pointer of bespoke blocks, just like for smaller allocations, to protect the management struct. Also update next_free to mark the block as full. This is only strictly necessary for the first allocated block, because subsequent ones are inserted after the current block and never considered for further allocations, but it's easier to just do it in all cases. Add a basic unit test to demonstrate the issue by using mem_pool_calloc() with a tiny block size, which forces the creation of a bespoke block. Helped-by: Phillip Wood Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

treewide: remove unnecessary cache.h includes in source files

2023-02-24T01:25:28Z

We had several C files include cache.h unnecessarily. Replace those with an include of "git-compat-util.h" instead. Much like the previous commit, these have all been verified via both ensuring that gcc -E $SOURCE_FILE | grep '"cache.h"' found no hits and that make DEVELOPER=1 ${OBJECT_FILE_FOR_SOURCE_FILE} successfully compiles without warnings. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

mem-pool: don't assume uintmax_t is aligned enough for all types

2022-01-24T18:26:40Z

mem_pool_alloc uses sizeof(uintmax_t) as a proxy for what should be _Alignof(max_align_t) in C11. On most architectures this is sufficient (though on m68k it is in fact overly strict, since the de-facto ABI, which differs from the specified System V ABI, has the maximum alignment of all types as 2 bytes), but on CHERI, and thus Arm's Morello prototype, it is insufficient for any type that stores a pointer, which must be aligned to 128 bits (on 64-bit architectures extended with CHERI), whilst uintmax_t is a 64-bit integer. Fix this by introducing our own approximation for max_align_t and a means to compute _Alignof it without relying on C11. Currently this union only contains uintmax_t and void *, but more types can be added as needed. Signed-off-by: Jessica Clarke Signed-off-by: Junio C Hamano

mem-pool: drop trailing semicolon from macro definition

2021-03-17T17:20:16Z

Allow BLOCK_GROWTH_SIZE to be used like an integer literal by removing the trailing semicolon from its definition. Also wrap the expression in parentheses, to allow it to be used with operators without leading to unexpected results. It doesn't matter for the current use site, but make it follow standard macro rules anyway to avoid future surprises. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

mem-pool: use consistent pool variable name

2020-08-18T19:16:08Z

About half the function declarations in mem-pool.h used 'struct mem_pool *pool', while the other half used 'struct mem_pool *mem_pool'. Make the code a bit more consistent by just using 'pool' in preference to 'mem_pool' everywhere. No behavioral changes included; this is just a mechanical rename (though a line or two was rewrapped as well). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

mem-pool: use more standard initialization and finalization

2020-08-18T19:16:06Z

A typical memory type, such as strbuf, hashmap, or string_list can be stored on the stack or embedded within another structure. mem_pool cannot be, because of how mem_pool_init() and mem_pool_discard() are written. mem_pool_init() does essentially the following (simplified for purposes of explanation here): void mem_pool_init(struct mem_pool **pool...) { *pool = xcalloc(1, sizeof(*pool)); It seems weird to require that mem_pools can only be accessed through a pointer. It also seems slightly dangerous: unlike strbuf_release() or strbuf_reset() or string_list_clear(), all of which put the data structure into a state where it can be re-used after the call, mem_pool_discard(pool) will leave pool pointing at free'd memory. read-cache (and split-index) are the only current users of mem_pools, and they haven't fallen into a use-after-free mistake here, but it seems likely to be problematic for future users especially since several of the current callers of mem_pool_init() will only call it when the mem_pool* is not already allocated (i.e. is NULL). This type of mechanism also prevents finding synchronization points where one can free existing memory and then resume more operations. It would be natural at such points to run something like mem_pool_discard(pool...); and, if necessary, mem_pool_init(&pool...); and then carry on continuing to use the pool. However, this fails badly if several objects had a copy of the value of pool from before these commands; in such a case, those objects won't get the updated value of pool that mem_pool_init() overwrites pool with and they'll all instead be reading and writing from free'd memory. Modify mem_pool_init()/mem_pool_discard() to behave more like strbuf_init()/strbuf_release() or string_list_init()/string_list_clear() In particular: (1) make mem_pool_init() just take a mem_pool* and have it only worry about allocating struct mp_blocks, not the struct mem_pool itself, (2) make mem_pool_discard() free the memory that the pool was responsible for, but leave it in a state where it can be used to allocate more memory afterward (without the need to call mem_pool_init() again). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano