Re: [PATCHv2 5/5] refs.c: enable large transactions

[Stefan Beller <sbeller@google.com>]

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:
> 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
>
>
>