Re: [PATCH v3 2/3] Introduce GIT_MMAP_LIMIT to allow testing expected mmap size
On Aug 22, 2014, at 12:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> Steffen Prohaska <prohaska@zib.de> writes:
>
>> Similar to testing expectations about malloc with GIT_ALLOC_LIMIT (see
>> commit d41489), it can be useful to test expectations about mmap.
>>
>> This introduces a new environment variable GIT_MMAP_LIMIT to limit the
>> largest allowed mmap length (in KB). xmmap() is modified to check the
>> limit. Together with GIT_ALLOC_LIMIT tests can now easily confirm
>> expectations about memory consumption.
>>
>> GIT_ALLOC_LIMIT will be used in the next commit to test that data will
>
> I smell the need for s/ALLOC/MMAP/ here, but perhaps you did mean
> ALLOC (I won't know until I check 3/3 ;-)
Show 15 quoted lines
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 00c07f2..88d64c0 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -663,10 +663,25 @@ void release_pack_memory(size_t need)
>> ; /* nothing */
>> }
>>
>> +static void mmap_limit_check(size_t length)
>> +{
>> + static int limit = -1;
>
> Perhaps you want ssize_t here? I see mmap() as a tool to handle a
> lot more data than a single malloc() typically would ;-) so previous
> mistakes by other people would not be a good excuse.>> + if (limit == -1) {
>> + const char *env = getenv("GIT_MMAP_LIMIT");
>> + limit = env ? atoi(env) * 1024 : 0;... this should then be changed to atol(env), and ...
>> + }
>> + if (limit && length > limit)
>> + die("attempting to mmap %"PRIuMAX" over limit %d",
>> + (intmax_t)length, limit);... here PRIuMAX and (uintmax_t); (uintmax_t) also for length.
I'll fix it and also GIT_ALLOC_LIMIT.
Steffen
Show 13 quoted lines
>> +}
>> +
>> void *xmmap(void *start, size_t length,
>> int prot, int flags, int fd, off_t offset)
>> {
>> - void *ret = mmap(start, length, prot, flags, fd, offset);
>> + void *ret;
>> +
>> + mmap_limit_check(length);
>> + ret = mmap(start, length, prot, flags, fd, offset);
>> if (ret == MAP_FAILED) {
>> if (!length)
>> return NULL;Steffen Prohaska <prohaska@zib.de>