Re: [PATCH v4 10/32] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN

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

 



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.

Michael

-- 
Michael Haggerty
mhagger@xxxxxxxxxxxx

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