Re: [PATCHv2 5/5] refs.c: enable large transactions
- From
Stefan Beller <sbeller@google.com>
- Date
- Jan 22, 2015, 19:16 UTC
- Message-ID
- <CAGZ79kZnECcGOWKr4JLpF5zh68+L8XX7yYntyZDJTbZ5cLD50w@mail.gmail.com>
- In-Reply-To
- <54C0F41F.2080705@ramsay1.demon.co.uk>
How do you run sparse on git?
I noticed there is 'make sparse' though I cannot get it working here in the corporate world as I have problems with openssl headers not being found.
Also the line numbers seem to bit off compared to what I have here, did you need to modify/preprocess files to get sparse running?
As for the fix, would it be sufficient to check != NULL instead of < 0?
Thanks, Stefan
On Thu, Jan 22, 2015 at 4:59 AM, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:
Show 90 quoted lines
> On 22/01/15 02:32, Stefan Beller wrote:
>> By closing the file descriptors after creating the lock file we are not
>> limiting the size of the transaction by the number of available file
>> descriptors.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> refs.c | 17 +++++++++++++----
>> t/t1400-update-ref.sh | 4 ++--
>> 2 files changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index 2013d37..9d01102 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3055,11 +3055,18 @@ int is_branch(const char *refname)
>> static int write_sha1_to_lock_file(struct ref_lock *lock,
>> const unsigned char *sha1)
>> {
>> - if (fdopen_lock_file(lock->lk, "w") < 0
>> - || fprintf(lock->lk->fp, "%s\n", sha1_to_hex(sha1)) != 41)
>> + if (lock->lk->fd == -1) {
>> + if (reopen_lock_file(lock->lk) < 0
>> + || fdopen_lock_file(lock->lk, "w") < 0
>
> fdopen_lock_file() returns a 'FILE *', so this causes sparse to bark:
>
> refs.c:3105:56: error: incompatible types for operation (<)
> refs.c:3105:56: left side has type struct _IO_FILE [usertype] *
> refs.c:3105:56: right side has type int
>
>> + || fprintf(lock->lk->fp, "%s\n", sha1_to_hex(sha1)) != 41
>> + || close_lock_file(lock->lk) < 0)
>> + return -1;
>> + } else {
>> + if (fdopen_lock_file(lock->lk, "w") < 0
>
> Similarly, sparse barks:
>
> refs.c:3110:53: error: incompatible types for operation (<)
> refs.c:3110:53: left side has type struct _IO_FILE [usertype] *
> refs.c:3110:53: right side has type int
>
>> + || fprintf(lock->lk->fp, "%s\n", sha1_to_hex(sha1)) != 41)
>> return -1;
>> - else
>> - return 0;
>> + }
>> + return 0;
>> }
>>
>> /*
>> @@ -3761,6 +3768,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,
>> update->refname);
>> goto cleanup;
>> }
>> + /* Do not keep all lock files open at the same time. */
>> + close_lock_file(update->lock->lk);
>> }
>>
>> /* Perform updates first so live commits remain referenced */
>> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
>> index 47d2fe9..c593a1d 100755
>> --- a/t/t1400-update-ref.sh
>> +++ b/t/t1400-update-ref.sh
>> @@ -979,7 +979,7 @@ run_with_limited_open_files () {
>>
>> test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
>>
>> -test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches does not burst open file limit' '
>> +test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches does not burst open file limit' '
>> (
>> for i in $(test_seq 33)
>> do
>> @@ -990,7 +990,7 @@ test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches
>> )
>> '
>>
>> -test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting branches does not burst open file limit' '
>> +test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting branches does not burst open file limit' '
>> (
>> for i in $(test_seq 33)
>> do
>>
>
> ATB,
> Ramsay Jones
>
>
>