Subject: Re: [PATCH v5 10/35] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN

From: Jonathan Nieder <[email-masked]>
Date: Tue, 16 Sep 2014 14:05:38 -0700

Michael Haggerty wrote:

> There are a few places that use these values, so define constants for
> them.

Seems like a symptom of the API leaving out a useful helper (e.g.,
something that strips off the lock suffix and returns a memdupz'd
filename).

[...]
> --- a/cache.h
> +++ b/cache.h
> @@ -570,6 +570,10 @@ extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
>  #define REFRESH_IN_PORCELAIN	0x0020	/* user friendly output, not "needs update" */
>  extern int refresh_index(struct index_state *, unsigned int flags, const struct pathspec *pathspec, char *seen, const char *header_msg);
>  
> +/* String appended to a filename to derive the lockfile name: */
> +#define LOCK_SUFFIX ".lock"
> +#define LOCK_SUFFIX_LEN 5

My suspicion is that error handling would be better if fewer callers
outside of lockfile.c did the '- LOCK_SUFFIX_LEN' dance, so this seems
like a step in the wrong direction.

Adding constants in lockfile.c would make sense, though.

Thanks,
Jonathan