Re: [PATCH 2/2] git-p4: fix error handling in P4Unshelve.renameBranch()

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 20, 2022, 16:21 UTC
Message-ID: <xmqq5yjr4ux8.fsf@gitster.g>
In-Reply-To: <xmqqa6934v2c.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 23 quoted lines

>>  
>> -        found = True
>
> This has to be initialized to False, because ...
>
>>          for i in range(0, 1000):
>>              backup_branch_name = "{0}.{1}".format(branch_name, i)
>>              if not gitBranchExists(backup_branch_name):
>>                  # Copy ref to backup
>>                  gitUpdateRef(backup_branch_name, branch_name)
>>                  gitDeleteRef(branch_name)
>> -                found = True
>>                  print("renamed old unshelve branch to {0}".format(backup_branch_name))
>
> ... we flip it to True when we find an available unused name and
> break out ...
>
>>                  break
>> -
>> -        if not found:
>> -            sys.exit("gave up trying to rename existing branch {0}".format(sync.branch))
>
> ... so that we can complain when we didn't find anything usable.

By the way, isn't this a typical time-of-check to time-of-use bug? Not the problem with the fix in the patch but in the original, but regardless of whose fault it is, it may be good to fix it (outside the topic of this patch).

Thanks.