All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ramsay Jones <ramsay@ramsayjones.plus.com>
To: David Turner <dturner@twopensource.com>,
	Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
	GIT Mailing-list <git@vger.kernel.org>
Subject: [PATCH 0/3] index-helper: fix UNIX_PATH_MAX redefinition error on cygwin
Date: Sun, 10 Apr 2016 23:57:31 +0100	[thread overview]
Message-ID: <570ADA5B.5030408@ramsayjones.plus.com> (raw)


Hi David, Duy,

If you need to re-roll your 'dt/index-helper' branch, could you please
consider squashing these patches into the relevant patch (equivalent to
commit 12909da4 ("index-helper: new daemon for caching index and related
stuff", 06-04-2016)).

The above commit causes the cygwin build to throw a warning (many times)
about the redefinition of the UNIX_PATH_MAX macro, viz:

      CC credential-store.o
  In file included from cache.h:4:0,
                   from credential-store.c:1:
  git-compat-util.h:1060:0: warning: "UNIX_PATH_MAX" redefined
   #define UNIX_PATH_MAX 92
   ^
  In file included from git-compat-util.h:212:0,
                   from cache.h:4,
                   from credential-store.c:1:
  /usr/include/sys/un.h:18:0: note: this is the location of the previous definition
   #define UNIX_PATH_MAX   108
   ^

Which is quite easy to confirm:

  $ find /usr/include -iname '*.h' | xargs grep -n UNIX_PATH_MAX
  /usr/include/sys/un.h:18:#define UNIX_PATH_MAX   108
  /usr/include/sys/un.h:22:  char	         sun_path[UNIX_PATH_MAX]; /* 108 bytes of socket address     */

  $ gcc -dD -E git-compat-util.h 2>/dev/null | grep -n 'un\.h'
  8716:# 1 "/usr/include/sys/un.h" 1 3 4
  8717:# 12 "/usr/include/sys/un.h" 3 4

  $ gcc -dD -E git-compat-util.h 2>/dev/null | grep -n 'UNIX_PATH_MAX'
  8724:#define UNIX_PATH_MAX 108
  32675:#define UNIX_PATH_MAX 92

  $ 

So, a simple fix would look like:

  diff --git a/git-compat-util.h b/git-compat-util.h
  index 0e35c13..6d65132 100644
  --- a/git-compat-util.h
  +++ b/git-compat-util.h
  @@ -1043,7 +1043,7 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
   #define getc_unlocked(fh) getc(fh)
   #endif
 
  -#ifdef __linux__
  +#if defined(__linux__) || defined(__CYGWIN__)
   #define UNIX_PATH_MAX 108
   #elif defined(__APPLE__) || defined(BSD)
   #define UNIX_PATH_MAX 104

... since you are allowed to redefine a macro to the same token sequence
(modulo some strange whitespace rules).

However, assuming the <sys/un.h> header is defining the UNIX_PATH_MAX
macro, why even try to #define it again. So, maybe put an

  #ifndef UNIX_PATH_MAX
  ...
  #endif

around the entire block. But then, why on earth are you doing this at all?

Let's look on linux:

  $ find /usr/include -iname '*.h' | xargs grep -n UNIX_PATH_MAX
  /usr/include/valgrind/vki/vki-linux.h:754:#define VKI_UNIX_PATH_MAX	108
  /usr/include/valgrind/vki/vki-linux.h:758:	char sun_path[VKI_UNIX_PATH_MAX];	/* pathname */
  /usr/include/linux/un.h:6:#define UNIX_PATH_MAX	108
  /usr/include/linux/un.h:10:	char sun_path[UNIX_PATH_MAX];	/* pathname */

  $ gcc -dD -E git-compat-util.h | grep -n 'un\.h'
  14119:# 1 "/usr/include/x86_64-linux-gnu/sys/un.h" 1 3 4
  14120:# 19 "/usr/include/x86_64-linux-gnu/sys/un.h" 3 4

  $ gcc -dD -E git-compat-util.h | grep -n 'UNIX_PATH_MAX'
  36788:#define UNIX_PATH_MAX 108

  $ 

Ah, interesting, ... despite having a header that does indeed define the
UNIX_PATH_MAX macro, the compiler is not using that header, but another
one which doesn't. Well, blow me down. ;-)

So, the approach taken by patch #1 is to forget about UNIX_PATH_MAX and
simply use sizeof(address.sun_path) instead!

The second and third patches are simply 'mild suggestions' for other
minor issues I noticed while looking into these warnings.

ATB,
Ramsay Jones

Ramsay Jones (3):
  index-helper: fix UNIX_PATH_MAX redefinition error on cygwin
  index-helper: convert strncpy to memcpy
  index-helper: take extra care with readlink

 git-compat-util.h | 17 -----------------
 index-helper.c    |  4 ++--
 read-cache.c      | 15 +++++++++++++--
 3 files changed, 15 insertions(+), 21 deletions(-)

-- 
2.8.0

             reply	other threads:[~2016-04-10 22:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-10 22:57 Ramsay Jones [this message]
2016-04-11 13:33 ` [PATCH 0/3] index-helper: fix UNIX_PATH_MAX redefinition error on cygwin Jeff King
2016-04-11 21:29   ` David Turner
2016-04-12  1:46     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=570ADA5B.5030408@ramsayjones.plus.com \
    --to=ramsay@ramsayjones.plus.com \
    --cc=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.