On Fri, Sep 12, 2014 at 10:13 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> On 09/12/2014 12:42 AM, Ronnie Sahlberg wrote:
>> Maybe we should not have a public constant defined for the length :
>> +#define LOCK_SUFFIX_LEN 5
>>
>> since it encourages unsafe code like : (this was unsafe long before
>> your patch so not a regression)
>> + i = strlen(result_file) - LOCK_SUFFIX_LEN; /* .lock */
>> result_file[i] = 0;
>>
>>
>>
>> What about removing LOCK_SUFFIX_LEN from the public API and introduce
>> a helper function something like :
>>
>>
>> /* pointer to the character where the lock suffix starts */
>> char *lock_suffix_ptr_safe(const char *filename)
>> {
>> size_t len = strlen(filename);
>> if (len < 5)
>> die("BUG:...
>> if (strcmp(filename + len - 5, LOCK_SUFFIX)
>> die("BUG:...
>> return filename + len - 5;
>> }
>>
>> and use it instead?
>
> At the end of this patch series, LOCK_SUFFIX_LEN is only used in two
> places outside of lockfile.c:
>
> * In check_refname_component(), to ensure that no component of a
> reference name ends with ".lock". This only indirectly has anything to
> do with lockfiles.
>
> * In delete_ref_loose(), to derive the name of the loose reference file
> from the name of the lockfile. It immediately xmemdupz()s the part of
> the filename that it needs, so it is kosher.
>
> I will add a function get_locked_file_path() for the use of the second
> caller.
>
> I like being able to use the symbolic constant at the first caller, and
> it is not dangerous. I don't think it is so important to make the
> constant private, because I think somebody programming sloppily wouldn't
> be deterred for long by not seeing a symbolic constant for the suffix
> length. So if it's OK with you I'll leave the constant.
It is OK with me.
--
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