Re: [PATCH 2/6] path: Make the 'get_st_mode_bits' symbol a file static

From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Apr 29, 2013, 22:53 UTC
Message-ID: <517EF9D4.50105@ramsay1.demon.co.uk>
In-Reply-To: <7vip36qxui.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 23 quoted lines

> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> On MinGW, sparse issues an "'get_st_mode_bits' not declared. Should
>> it be static?" warning. The MinGW and MSVC builds do not see the
>> declaration of this function, within git-compat-util.h, due to it's
>> placement within an preprocessor conditional. (So, one solution would
>> be to simply move the declaration to the top level of the header.)
> 
> Well, the idea was that the user of this function in path.c will
> call get_st_mode_bits(), and whatever platform that provides a
> replacement implementation would do
> 
> 	#define get_st_mode_bits(a,b) cygwin_get_st_mode_bits((a),(b))
> 
> so that the calling site in path.c will end up calling that
> replacement implementation.  So if anything get_st_mode_bits()
> declaration may want to go at the _end_ (not top) after including
> all the compatibility crufts.
> 
> We could make the declaration static to path.c, but then nobody
> other than path.c would be able to make use of it in the future,
> and we'll have the same discussion when somebody wants to hoist the
> declaration to git-compat-util.h, no?

I don't have a problem with keeping the declaration in git-compat-util.h (after moving it so that the MinGW and MSVC builds can see it, of course) if you would rather do that.

However, I'm always a little wary when I hear someone say "this may be useful to others in the future, so lets do X to make it easier ...". I have noticed that, much more often than not, that future user never does materialise ... ;-)

Having said that ...

Back in 2011, when I was trying to fix t7606-merge-custom.sh (see commit b8a9733377, "help.c: Fix detection of custom merge strategy on cygwin", 16-06-2011), I had a patch that looked very similar to the solution Torsten has arrived at here! (The main difference was that I called stat() rather than lstat() to get the mode bits).

I decided to go with the simpler solution in commit b8a9733377.

ATB, Ramsay Jones