On Aug 22, 2014, at 12:26 AM, Junio C Hamano <gits...@pobox.com> wrote:
> Steffen Prohaska <proha...@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 ;-)
You are right.
>> 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.
Maybe; and ...
>> + 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
>> +}
>> +
>> 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;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
