Re: [PATCH v3 2/3] Introduce GIT_MMAP_LIMIT to allow testing expected mmap size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Aug 22, 2014, at 12:26 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:

> Steffen Prohaska <prohaska@xxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]