threads / discuss / 3094

cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

Subject: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

## tl;dr

27 messages between Jan 18, 2006 and Jan 23, 2006.

replies: 26people: 5as markdown or json

Alex Riesen· Jan 18, 2006, 13:47 UTC · lore

For reasons unknown, cygwin decided to use our sockaddr_storage. As it is redefined to sockaddr_in it'd cause compilation errors in cygwin headers. Fixed by first patch, which uses a more git-related name (can we claim rights for the symbol, being first to use it? :-)

For the other, probably unrelated, reasons, they decided to leave declarations of DT_* macros in dirent.h without providing dirent->d_type. This is what NO_DTYPE in the 0002-patch is all about.

And on top of that, they removed dirent->d_ino (or probably replaced it by __ino32, if at all). BTW, can we somehow avoid using d_ino? It is referenced only in fsck-objects.c Anyway, to workaround this I put

  COMPAT_CFLAGS += -Dd_ino=__ino32
It helps, but surely is not the solution.
P.S. For all you poor souls on Win2k, my config.mak contains:
  NO_MMAP = YesPlease
  NO_DTYPE = YesPlease
  COMPAT_CFLAGS += -Dd_ino=__ino32
Just to make it work (at least it's enough for me, even the tests run...)
>From nobody Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Wed Jan 18 14:02:54 2006 +0100
Subject: fix compilation of sockaddr_storage
---
 Makefile |    2 +-
 daemon.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
59379c380a6c2829c5614aadd4a5492abb8d14c8
diff --git a/Makefile b/Makefile
index f6d9e0a..5782e2a 100644
--- a/Makefile
+++ b/Makefile
@@ -342,7 +342,7 @@ ifdef NO_MMAP
 	COMPAT_OBJS += compat/mmap.o
 endif
 ifdef NO_IPV6
-	ALL_CFLAGS += -DNO_IPV6 -Dsockaddr_storage=sockaddr_in
+	ALL_CFLAGS += -DNO_IPV6 -Dsockaddr_stg_git=sockaddr_in
 endif
 
 ifdef PPC_SHA1
diff --git a/daemon.c b/daemon.c
index bb014fa..b424e5e 100644
--- a/daemon.c
+++ b/daemon.c
@@ -277,7 +277,7 @@ static unsigned int children_deleted = 0
 static struct child {
 	pid_t pid;
 	int addrlen;
-	struct sockaddr_storage address;
+	struct sockaddr_stg_git address;
 } live_child[MAX_CHILDREN];
 
 static void add_child(int idx, pid_t pid, struct sockaddr *addr, int addrlen)
@@ -578,7 +578,7 @@ static int service_loop(int socknum, int
 
 		for (i = 0; i < socknum; i++) {
 			if (pfd[i].revents & POLLIN) {
-				struct sockaddr_storage ss;
+				struct sockaddr_stg_git ss;
 				unsigned int sslen = sizeof(ss);
 				int incoming = accept(pfd[i].fd, (struct sockaddr *)&ss, &sslen);
 				if (incoming < 0) {
-- 
1.1.2-ge577





>From nobody Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Wed Jan 18 14:04:34 2006 +0100
Subject: fix compilation of dirent->d_type

---

 Makefile |    3 +++
 cache.h  |   10 +++++++++-
 2 files changed, 12 insertions(+), 1 deletions(-)

5232f9128ad9d10368525fe709954acc2d6f49b3
diff --git a/Makefile b/Makefile
index 5782e2a..c3bae0f 100644
--- a/Makefile
+++ b/Makefile
@@ -329,6 +329,9 @@ ifdef NEEDS_NSL
 	LIBS += -lnsl
 	SIMPLE_LIB += -lnsl
 endif
+ifdef NO_DTYPE
+	COMPAT_CFLAGS += -DNO_DTYPE
+endif
 ifdef NO_STRCASESTR
 	COMPAT_CFLAGS += -DNO_STRCASESTR
 	COMPAT_OBJS += compat/strcasestr.o
diff --git a/cache.h b/cache.h
index 29c9e81..3846fb9 100644
--- a/cache.h
+++ b/cache.h
@@ -10,13 +10,21 @@
 #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)
 #endif
 
-#ifdef DT_UNKNOWN
+#ifndef NO_DTYPE
 #define DTYPE(de)	((de)->d_type)
 #else
+#ifndef DT_UNKNOWN
 #define DT_UNKNOWN	0
+#endif
+#ifndef DT_DIR
 #define DT_DIR		1
+#endif
+#ifndef DT_REG
 #define DT_REG		2
+#endif
+#ifndef DT_LNK
 #define DT_LNK		3
+#endif
 #define DTYPE(de)	DT_UNKNOWN
 #endif
 
-- 
1.1.2-ge577
Christopher Faylor· Jan 19, 2006, 05:29 UTC · re: Alex Riesen · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On Wed, Jan 18, 2006 at 02:47:00PM +0100, Alex Riesen wrote:
Show 8 quoted lines
>For reasons unknown, cygwin decided to use our sockaddr_storage.
>As it is redefined to sockaddr_in it'd cause compilation errors in
>cygwin headers. Fixed by first patch, which uses a more git-related
>name (can we claim rights for the symbol, being first to use it? :-)
>
>For the other, probably unrelated, reasons, they decided to leave
>declarations of DT_* macros in dirent.h without providing dirent->d_type.
>This is what NO_DTYPE in the 0002-patch is all about.

"They" probably would like to hear about any irregularities that are found. "They" probably don't like it when people treat an open source project as if it was some unresponsive proprietary enterprise which does not listen to or accept patches.

Show 7 quoted lines
>And on top of that, they removed dirent->d_ino (or probably replaced it
>by __ino32, if at all).  BTW, can we somehow avoid using d_ino?  It is
>referenced only in fsck-objects.c Anyway, to workaround this I put
>
>COMPAT_CFLAGS += -Dd_ino=__ino32
>
>It helps, but surely is not the solution.

I don't see how it could help since __ino32 is not actually filled in with anything. In fact, I'll rename the field to __invalid_ino32 to make that clear.

cgf (cygwin project lead)

Junio C Hamano· Jan 19, 2006, 08:59 UTC · re: Christopher Faylor · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

Christopher Faylor <me@cgf.cx> writes:
> "They" probably would like to hear about any irregularities that are found.
> "They" probably don't like it when people treat an open source project as
> if it was some unresponsive proprietary enterprise which does not listen
> to or accept patches.

First of all, thanks for joining our discussion. Being able to hear from somebody from other project firsthand (not just listen to somebody talking in his own changelogs and code comments, but in actual e-mail exchange discussion) lets us put faces and names to the entity "so far just one of the external projects to us".

>>For reasons unknown, cygwin decided to use our sockaddr_storage.

I haven't looked at the proposed patch by Alex, so would not comment on this part, but I'd appreciate your input.

>>For the other, probably unrelated, reasons, they decided to leave
>>declarations of DT_* macros in dirent.h without providing dirent->d_type.

I was wondering what the justification for keeping DT_* without d_type myself. What is the preferred resolution on this one from your point of view? I suspect removing d_type while leaving DT_* was just a transient error and you would want to remove DT_* as well, in which case the patch on this issue by Alex would become unnecessary.

Show 11 quoted lines
>>And on top of that, they removed dirent->d_ino (or probably replaced it
>>by __ino32, if at all).  BTW, can we somehow avoid using d_ino?  It is
>>referenced only in fsck-objects.c Anyway, to workaround this I put
>>
>>COMPAT_CFLAGS += -Dd_ino=__ino32
>>
>>It helps, but surely is not the solution.
>
> I don't see how it could help since __ino32 is not actually filled in
> with anything.  In fact, I'll rename the field to __invalid_ino32 to
> make that clear.

I think renaming __invalid_* makes sense. I'll see how we would work this around on the git side to make things more portable.

Christopher Faylor· Jan 19, 2006, 16:10 UTC · re: Junio C Hamano · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On Thu, Jan 19, 2006 at 12:59:57AM -0800, Junio C Hamano wrote:
Show 7 quoted lines
>Christopher Faylor <me@cgf.cx> writes:
>>"They" probably would like to hear about any irregularities that are
>>found.  "They" probably don't like it when people treat an open source
>>project as if it was some unresponsive proprietary enterprise which
>>does not listen to or accept patches.
>
>First of all, thanks for joining our discussion.
You're welcome.  I use git on linux and cygwin so I'm happy to try to help.
>Being able to hear from somebody from other project firsthand (not just
>listen to somebody talking in his own changelogs and code comments, but
>in actual e-mail exchange discussion) lets us put faces and names to
>the entity "so far just one of the external projects to us".

I only read this list sporadically but I have chimed in from time to time when people were talking about cygwin. For the most part, it seems like my input hasn't been needed all that much.

Show 14 quoted lines
>>>For reasons unknown, cygwin decided to use our sockaddr_storage.
>
>I haven't looked at the proposed patch by Alex, so would not comment on
>this part, but I'd appreciate your input.
>
>>>For the other, probably unrelated, reasons, they decided to leave
>>>declarations of DT_* macros in dirent.h without providing
>>>dirent->d_type.
>
>I was wondering what the justification for keeping DT_* without d_type
>myself.  What is the preferred resolution on this one from your point
>of view?  I suspect removing d_type while leaving DT_* was just a
>transient error and you would want to remove DT_* as well, in which
>case the patch on this issue by Alex would become unnecessary.

Actually, I started adding DT_* macros at one point, in preparation for adding d_type, and then got sidetracked. Their existence is a real bug so, I have ifdef'ed out the DT macros in current CVS.

Show 14 quoted lines
>>>And on top of that, they removed dirent->d_ino (or probably replaced it
>>>by __ino32, if at all).  BTW, can we somehow avoid using d_ino?  It is
>>>referenced only in fsck-objects.c Anyway, to workaround this I put
>>>
>>>COMPAT_CFLAGS += -Dd_ino=__ino32
>>>
>>>It helps, but surely is not the solution.
>>
>> I don't see how it could help since __ino32 is not actually filled in
>> with anything.  In fact, I'll rename the field to __invalid_ino32 to
>> make that clear.
>
>I think renaming __invalid_* makes sense.  I'll see how we would
>work this around on the git side to make things more portable.

There were two fields in the dirent struct. One was the "real" d_ino which was a 64-bit ino_t, __ino32 was a legacy field from a time when inodes were 32 bits. I had already renamed the d_ino to __invalid_d_ino but I didn't think I had to rename __ino32, too, since it wasn't a standard field and didn't think that anyone would be using it. However, it is now __invalid_ino32 (and will probably disappear entirely) in CVS.

I knew that there would be fallout from getting rid of d_ino but this change has been a long time coming. Previously, the inodes reported in d_ino were different from the (correct) ones in st_ino and some applications were understandbly confused by that fact. Making d_ino accurate would have meant that we'd have to open every file in readdir to get the windows equivalent of inode information and, since we get enough "cygwin is slow" complaints, that wasn't a cost we were willing to pay.

cgf
Christopher Faylor· Jan 19, 2006, 20:34 UTC · re: Christopher Faylor · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On Thu, Jan 19, 2006 at 11:10:00AM -0500, Christopher Faylor wrote:
Show 10 quoted lines
>On Thu, Jan 19, 2006 at 12:59:57AM -0800, Junio C Hamano wrote:
>>Christopher Faylor <me@cgf.cx> writes:
>>>"They" probably would like to hear about any irregularities that are
>>>found.  "They" probably don't like it when people treat an open source
>>>project as if it was some unresponsive proprietary enterprise which
>>>does not listen to or accept patches.
>>
>>First of all, thanks for joining our discussion.
>
>You're welcome.  I use git on linux and cygwin so I'm happy to try to help.

Btw, we're looking to roll out a new release of cygwin which fixes the embarrassing typo in sockaddr_storage. It is fixed in cygwin snapshots:

http://cygwin.com/snapshots/
cgf
Linus Torvalds· Jan 19, 2006, 21:16 UTC · re: Christopher Faylor · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On Thu, 19 Jan 2006, Christopher Faylor wrote:
Show 5 quoted lines
> 
> Btw, we're looking to roll out a new release of cygwin which fixes the
> embarrassing typo in sockaddr_storage.  It is fixed in cygwin snapshots:
> 
> http://cygwin.com/snapshots/

Quick question for cygwin people (I asked this at an earlier point, but I don't think there was any reply): would cygwin prefer using "vfork()" over "fork()", or is there no advantage? With vfork(), I could imagine that you might avoid a lot of strange VM games..

I think almost all of the git fork usage is of the type where "vfork()" would work fine (git-daemon in non-inetd form is an exception, perhaps the only one).

So if using vfork() is preferable, we probably should do that. It tends to be a small performance improvement on Linux too, although on Linux it's really pretty much in the noise.

Alternatively, is there anything else we can do that makes things easier?
			Linus
Christopher Faylor· Jan 19, 2006, 21:28 UTC · re: Linus Torvalds · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On Thu, Jan 19, 2006 at 01:16:17PM -0800, Linus Torvalds wrote:
Show 11 quoted lines
>On Thu, 19 Jan 2006, Christopher Faylor wrote:
>>Btw, we're looking to roll out a new release of cygwin which fixes the
>>embarrassing typo in sockaddr_storage.  It is fixed in cygwin
>>snapshots:
>>
>>http://cygwin.com/snapshots/
>
>Quick question for cygwin people (I asked this at an earlier point, but
>I don't think there was any reply): would cygwin prefer using "vfork()"
>over "fork()", or is there no advantage?  With vfork(), I could imagine
>that you might avoid a lot of strange VM games..

Sorry, I missed the earlier question. I have to get in the habit of scanning this list more regularly for this type of thing.

Cygwin's vfork implementation currently defaults to fork so it doesn't matter which is used.

We used to have a vfork which tried to cut down on some of the substantial overhead that comes with cygwin's fork() but the vfork implementation eventually grew so complicated that there was eventually no performance gain and I decided to just yank it and revisit it at a later point.

So, for now, there is no difference, but, eventually, there might be if someone masters courage to revisit vfork-on-cygwin.

So, I guess that means that it would be a good idea to switch to vfork if you were planning for the nebulous future when this made a difference to cygwin. Otherwise, I wouldn't bother.

>Alternatively, is there anything else we can do that makes things easier?

I'm really committed to making cygwin as much like linux as possible so that you won't have to make things easier. The last release added some stuff which should make building linux easier, in fact. Also, the mmap() implementation should be a littler closer to linux. It is, of course, a work in progress, though.

The only thing that would speed up process creation in cygwin now is the use of the windows spawn* family of function calls. Those could be used instead of fork/exec but I have a personal aversion to using them since they are so non-UNIX. If performance is an issue, however, that could be something to investigate.

cgf
Linus Torvalds· Jan 19, 2006, 21:44 UTC · re: Christopher Faylor · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On Thu, 19 Jan 2006, Christopher Faylor wrote:
Show 6 quoted lines
> 
> The only thing that would speed up process creation in cygwin now is
> the use of the windows spawn* family of function calls.  Those could be
> used instead of fork/exec but I have a personal aversion to using them
> since they are so non-UNIX.  If performance is an issue, however, that
> could be something to investigate.

What about posix_spawn()? I don't know the windows spawn things, but I assume posix_spawn() is basically trying to be them.. Does it work with cygwin if we try to convert some of the more obvious users?

Some of the git users should be easy to convert to posix_spawn()..
		Linus
Christopher Faylor· Jan 19, 2006, 21:51 UTC · re: Linus Torvalds · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On Thu, Jan 19, 2006 at 01:44:58PM -0800, Linus Torvalds wrote:
Show 12 quoted lines
>
>On Thu, 19 Jan 2006, Christopher Faylor wrote:
>> 
>> The only thing that would speed up process creation in cygwin now is
>> the use of the windows spawn* family of function calls.  Those could be
>> used instead of fork/exec but I have a personal aversion to using them
>> since they are so non-UNIX.  If performance is an issue, however, that
>> could be something to investigate.
>
>What about posix_spawn()? I don't know the windows spawn things, but I 
>assume posix_spawn() is basically trying to be them.. Does it work with 
>cygwin if we try to convert some of the more obvious users?

You know, I couldn't remember what the name of the spawn that was introduced for posix. I was going to mention it in my email and I forgot. Obviously it was because the name so obscure that no one could possibly remember it. :-)

We don't emulate posix_spawn currently but we could and should. I'll put it on the todo list.

cgf
Junio C Hamano· Jan 20, 2006, 01:13 UTC · re: Junio C Hamano · lore

[PATCH] fsck-objects: support platforms without d_ino in struct dirent.

The d_ino field is only used for performance reasons in fsck-objects. On a typical filesystem, i-number tends to have a strong correlation with where the actual bits sit on the disk platter, and we sort the entries to allow us scan things that ought to be close together together.

If the platform lacks support for it, it is not a big deal. Just do not use d_ino for sorting, and scan them unsorted.

---
    Junio C Hamano <junkio@cox.net> writes:
    > ...  I'll see how we would
    > work this around on the git side to make things more portable.
 Makefile       |    6 ++++++
 fsck-objects.c |   13 +++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)
4bec2123116a92fbeae12afc70317dfc85f6087b
diff --git a/Makefile b/Makefile
index e8f4b24..31898f7 100644
--- a/Makefile
+++ b/Makefile
@@ -19,6 +19,8 @@ all:
 # Define NO_EXPAT if you do not have expat installed.  git-http-push is
 # not built, and you cannot push using http:// and https:// transports.
 #
+# Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
+#
 # Define NO_STRCASESTR if you don't have strcasestr.
 #
 # Define NO_SETENV if you don't have setenv in the C library.
@@ -231,6 +233,7 @@ ifeq ($(uname_S),SunOS)
 	ALL_CFLAGS += -D__EXTENSIONS__
 endif
 ifeq ($(uname_O),Cygwin)
+	NO_D_INO_IN_DIRENT = YesPlease
 	NO_STRCASESTR = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	# There are conflicting reports about this.
@@ -332,6 +335,9 @@ ifdef NEEDS_NSL
 	LIBS += -lnsl
 	SIMPLE_LIB += -lnsl
 endif
+ifdef NO_D_INO_IN_DIRENT
+	ALL_CFLAGS += -DNO_D_INO_IN_DIRENT
+endif
 ifdef NO_STRCASESTR
 	COMPAT_CFLAGS += -DNO_STRCASESTR
 	COMPAT_OBJS += compat/strcasestr.o
diff --git a/fsck-objects.c b/fsck-objects.c
index 90e638e..9950be2 100644
--- a/fsck-objects.c
+++ b/fsck-objects.c
@@ -20,6 +20,13 @@ static int check_strict = 0;
 static int keep_cache_objects = 0; 
 static unsigned char head_sha1[20];
 
+#if NO_D_INO_IN_DIRENT
+#define SORT_DIRENT 0
+#define DIRENT_SORT_HINT(de) 0
+#else
+#define SORT_DIRENT 1
+#define DIRENT_SORT_HINT(de) ((de)->d_ino)
+#endif
 
 static void objreport(struct object *obj, const char *severity,
                       const char *err, va_list params)
@@ -307,7 +314,9 @@ static void fsck_sha1_list(void)
 {
 	int i, nr = sha1_list.nr;
 
-	qsort(sha1_list.entry, nr, sizeof(struct sha1_entry *), ino_compare);
+	if (SORT_DIRENT)
+		qsort(sha1_list.entry, nr,
+		      sizeof(struct sha1_entry *), ino_compare);
 	for (i = 0; i < nr; i++) {
 		struct sha1_entry *entry = sha1_list.entry[i];
 		unsigned char *sha1 = entry->sha1;
@@ -361,7 +370,7 @@ static int fsck_dir(int i, char *path)
 			memcpy(name+2, de->d_name, len+1);
 			if (get_sha1_hex(name, sha1) < 0)
 				break;
-			add_sha1_list(sha1, de->d_ino);
+			add_sha1_list(sha1, DIRENT_SORT_HINT(de));
 			continue;
 		}
 		fprintf(stderr, "bad sha1 file: %s/%s\n", path, de->d_name);
-- 
1.1.3-gacdd
Christopher Faylor· Jan 20, 2006, 03:38 UTC · re: Junio C Hamano · lore

Re: [PATCH] fsck-objects: support platforms without d_ino in struct dirent.

On Thu, Jan 19, 2006 at 05:13:51PM -0800, Junio C Hamano wrote:
Show 8 quoted lines
>The d_ino field is only used for performance reasons in
>fsck-objects.  On a typical filesystem, i-number tends to have a
>strong correlation with where the actual bits sit on the disk
>platter, and we sort the entries to allow us scan things that
>ought to be close together together.
>
>If the platform lacks support for it, it is not a big deal.
>Just do not use d_ino for sorting, and scan them unsorted.

Ah, interesting. I don't know if the file id objects would have that sort of correlation on NT but obviously the file hashes don't.

cgf
Alex Riesen· Jan 19, 2006, 10:42 UTC · re: Christopher Faylor · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On 1/19/06, Christopher Faylor <me@cgf.cx> wrote:
> "They" probably don't like it when people treat an open source project as
> if it was some unresponsive proprietary enterprise which does not listen
> to or accept patches.

Please, accept my appologies for the sarcasm in the original post. Sometimes I get an impression of cygwin being not maintained at all, and that, if not justifies my behavior, but at least is an attempt to explain it.

Show 11 quoted lines
> >And on top of that, they removed dirent->d_ino (or probably replaced it
> >by __ino32, if at all).  BTW, can we somehow avoid using d_ino?  It is
> >referenced only in fsck-objects.c Anyway, to workaround this I put
> >
> >COMPAT_CFLAGS += -Dd_ino=__ino32
> >
> >It helps, but surely is not the solution.
>
> I don't see how it could help since __ino32 is not actually filled in
> with anything.  In fact, I'll rename the field to __invalid_ino32 to
> make that clear.

But why keep the DT_-macros?! And why there is two fields hinting at d_ino, and why there is 3 (!) "struct dirent" definitions in dirent.h (sys/dirent.h)? Some with different names (d_reserved?). And if cygwin is aiming for posix, what would d_fd or d_version be (Open Group Specs v6[1] mention only d_ino and d_name)?

[1] http://www.opengroup.org/onlinepubs/009695399/basedefs/dirent.h.html
Christopher Faylor· Jan 19, 2006, 18:31 UTC · re: Alex Riesen · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On Thu, Jan 19, 2006 at 11:42:46AM +0100, Alex Riesen wrote:
Show 9 quoted lines
>On 1/19/06, Christopher Faylor <me@cgf.cx> wrote:
>>"They" probably don't like it when people treat an open source project
>>as if it was some unresponsive proprietary enterprise which does not
>>listen to or accept patches.
>
>Please, accept my appologies for the sarcasm in the original post.
>Sometimes I get an impression of cygwin being not maintained at all,
>and that, if not justifies my behavior, but at least is an attempt to
>explain it.

Hmm. I thought we'd already dispelled the myth that cygwin is unsupported in this very mailing list. That is an odd impression given the fact that you were complaining about behavior in a version of cygwin which was released on Monday but, apology accepted.

If you want to see evidence of continual cygwin development, you can always visit this page: http://cygwin.com/snapshots/ . This page has snapshots of cygwin built from cvs. We make these available so that people will check things out prior to an actual release.

Show 14 quoted lines
>>>And on top of that, they removed dirent->d_ino (or probably replaced it
>>>by __ino32, if at all).  BTW, can we somehow avoid using d_ino?  It is
>>>referenced only in fsck-objects.c Anyway, to workaround this I put
>>>
>>>COMPAT_CFLAGS += -Dd_ino=__ino32
>>>
>>>It helps, but surely is not the solution.
>>
>>I don't see how it could help since __ino32 is not actually filled in
>>with anything.  In fact, I'll rename the field to __invalid_ino32 to
>>make that clear.
>
>But why keep the DT_-macros?! And why there is two fields hinting at
>d_ino, and why there is 3 (!)

The default entry (i.e., the one you get without defining __INSIDE_CYGWIN__ or __CYGWIN_USE_BIG_TYPES__) in dirent.h is the correct one.

Show 7 quoted lines
>"struct dirent" definitions in dirent.h (sys/dirent.h)?  Some with
>different names (d_reserved?).  And if cygwin is aiming for posix, what
>would d_fd or d_version be (Open Group Specs v6[1] mention only d_ino
>and d_name)?
>
>[1]
>http://www.opengroup.org/onlinepubs/009695399/basedefs/dirent.h.html

Hmm. On linux, my /usr/include/bits/dirent.h has a d_reclen field in dirent. I know what that is and what it is used for but it's not mentioned, that I can see, in SUSv3. But, since I don't see anything in the description of dirent in SUSv3 which says that the must have only the fields mentiond, that's ok.

In any event, we don't claim to be POSIX compatible. We actually are working for linux compatibility but this is one regrettable place where Windows doesn't allow that.

I explained about the DT macros and why we dropped d_ino support in another message.

Anyway, I understand why the DT macros would cause problems and I have removed them from the current CVS. I don't see why the existence of extra fields in dirent or why other non-default definitions would cause any problems other than the "Doctor, doctor, it hurts when I do this" variety.

cgf
Alex Riesen· Jan 19, 2006, 22:08 UTC · re: Christopher Faylor · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

Christopher Faylor, Thu, Jan 19, 2006 19:31:43 +0100:
Show 12 quoted lines
> >>"They" probably don't like it when people treat an open source project
> >>as if it was some unresponsive proprietary enterprise which does not
> >>listen to or accept patches.
> >Please, accept my appologies for the sarcasm in the original post.
> >Sometimes I get an impression of cygwin being not maintained at all,
> >and that, if not justifies my behavior, but at least is an attempt to
> >explain it.
> 
> Hmm.  I thought we'd already dispelled the myth that cygwin is
> unsupported in this very mailing list.  That is an odd impression given
> the fact that you were complaining about behavior in a version of cygwin
> which was released on Monday but, apology accepted.

It was my first update since a long time (which BTW broke some programs like cp: they missed symbols in cygwin1.dll).

> If you want to see evidence of continual cygwin development, you can always
> visit this page:  http://cygwin.com/snapshots/ .  This page has snapshots
> of cygwin built from cvs.  We make these available so that people will check
> things out prior to an actual release.
Thanks.
Show 18 quoted lines
> >>>And on top of that, they removed dirent->d_ino (or probably replaced it
> >>>by __ino32, if at all).  BTW, can we somehow avoid using d_ino?  It is
> >>>referenced only in fsck-objects.c Anyway, to workaround this I put
> >>>
> >>>COMPAT_CFLAGS += -Dd_ino=__ino32
> >>>
> >>>It helps, but surely is not the solution.
> >>
> >>I don't see how it could help since __ino32 is not actually filled in
> >>with anything.  In fact, I'll rename the field to __invalid_ino32 to
> >>make that clear.
> >
> >But why keep the DT_-macros?! And why there is two fields hinting at
> >d_ino, and why there is 3 (!)
> 
> The default entry (i.e., the one you get without defining
> __INSIDE_CYGWIN__ or __CYGWIN_USE_BIG_TYPES__) in dirent.h is the
> correct one.

Maybe it'd be a good idea just to remove the definitions? Or, as __INSIDE_CYGWIN__ implies, move them into cygwin internal sources. Would be less confusion and no chance of someone defining one of the macros and getting a binary-incompatible object?

Show 13 quoted lines
> >"struct dirent" definitions in dirent.h (sys/dirent.h)?  Some with
> >different names (d_reserved?).  And if cygwin is aiming for posix, what
> >would d_fd or d_version be (Open Group Specs v6[1] mention only d_ino
> >and d_name)?
> >
> >[1]
> >http://www.opengroup.org/onlinepubs/009695399/basedefs/dirent.h.html
> 
> Hmm.  On linux, my /usr/include/bits/dirent.h has a d_reclen field in
> dirent.  I know what that is and what it is used for but it's not
> mentioned, that I can see, in SUSv3.  But, since I don't see anything in
> the description of dirent in SUSv3 which says that the must have only
> the fields mentiond, that's ok.
Of course, you don't have to. It all about making an impression
> In any event, we don't claim to be POSIX compatible.  We actually are
> working for linux compatibility but this is one regrettable place where
> Windows doesn't allow that.
The word was "aiming"
Show 5 quoted lines
> Anyway, I understand why the DT macros would cause problems and I have
> removed them from the current CVS.  I don't see why the existence of
> extra fields in dirent or why other non-default definitions would
> cause any problems other than the "Doctor, doctor, it hurts when I
> do this" variety.

It is not the existance of the extra fields which cause problems. It is an existance of fields, the names of which imply a functionality they do not provide which causes problems. Why should I seeing __ino32 in an official header think: "it is never filled anyway, so I shouldn't use it"?! Or what could "__invalid_d_ino" mean? If it is invalid (as in "can't be used", why is it there at all?

Christopher Faylor· Jan 19, 2006, 22:51 UTC · re: Alex Riesen · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On Thu, Jan 19, 2006 at 11:08:43PM +0100, Alex Riesen wrote:
Show 8 quoted lines
>Christopher Faylor, Thu, Jan 19, 2006 19:31:43 +0100:
>>Hmm.  I thought we'd already dispelled the myth that cygwin is
>>unsupported in this very mailing list.  That is an odd impression given
>>the fact that you were complaining about behavior in a version of
>>cygwin which was released on Monday but, apology accepted.
>
>It was my first update since a long time (which BTW broke some programs
>like cp: they missed symbols in cygwin1.dll).

Detailed bug reports are always welcome on the cygwin mailing list: cygwin at cygwin dot com . I should point out that the existence of bugs in a product does not indicate a lack of support of the product, however.

>Maybe it'd be a good idea just to remove the definitions? Or, as
>__INSIDE_CYGWIN__ implies, move them into cygwin internal sources.
>Would be less confusion and no chance of someone defining one of the
>macros and getting a binary-incompatible object?

I sincerely doubt that anyone in this mailing list wants to get into a discussion of cygwin design. If you really want to discuss this, the cygwin mailing list is a much better place for that.

cgf
Petr Baudis· Jan 19, 2006, 12:51 UTC · re: Alex Riesen · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

Dear diary, on Wed, Jan 18, 2006 at 02:47:00PM CET, I got a letter where Alex Riesen <raa.lkml@gmail.com> said that...

> For reasons unknown, cygwin decided to use our sockaddr_storage.
> As it is redefined to sockaddr_in it'd cause compilation errors in
> cygwin headers. Fixed by first patch, which uses a more git-related
> name (can we claim rights for the symbol, being first to use it? :-)
Huh? "Our"? See RFC 2553 and e.g.:
	http://www.opengroup.org/onlinepubs/009695399/basedefs/sys/socket.h.html

You have no business meddling with this identifier except working around platforms which do not support it, but then do not complain that things break when the platforms actually start supporting it. ;-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Of the 3 great composers Mozart tells us what it's like to be human,
Beethoven tells us what it's like to be Beethoven and Bach tells us
what it's like to be the universe.  -- Douglas Adams
Alex Riesen· Jan 19, 2006, 15:04 UTC · re: Petr Baudis · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On 1/19/06, Petr Baudis <pasky@suse.cz> wrote:
Show 14 quoted lines
> Dear diary, on Wed, Jan 18, 2006 at 02:47:00PM CET, I got a letter
> where Alex Riesen <raa.lkml@gmail.com> said that...
> > For reasons unknown, cygwin decided to use our sockaddr_storage.
> > As it is redefined to sockaddr_in it'd cause compilation errors in
> > cygwin headers. Fixed by first patch, which uses a more git-related
> > name (can we claim rights for the symbol, being first to use it? :-)
>
> Huh? "Our"? See RFC 2553 and e.g.:
>
>         http://www.opengroup.org/onlinepubs/009695399/basedefs/sys/socket.h.html
>
> You have no business meddling with this identifier except working around
> platforms which do not support it, but then do not complain that things
> break when the platforms actually start supporting it. ;-)
That will remind me to consult the specs first.
Petr Baudis· Jan 19, 2006, 13:00 UTC · re: Alex Riesen · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

I actually forgot to look at the patch.

Dear diary, on Wed, Jan 18, 2006 at 02:47:00PM CET, I got a letter where Alex Riesen <raa.lkml@gmail.com> said that...

Show 15 quoted lines
>  Makefile |    2 +-
>  daemon.c |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> 59379c380a6c2829c5614aadd4a5492abb8d14c8
> diff --git a/Makefile b/Makefile
> index f6d9e0a..5782e2a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -342,7 +342,7 @@ ifdef NO_MMAP
>  	COMPAT_OBJS += compat/mmap.o
>  endif
>  ifdef NO_IPV6
> -	ALL_CFLAGS += -DNO_IPV6 -Dsockaddr_storage=sockaddr_in
> +	ALL_CFLAGS += -DNO_IPV6 -Dsockaddr_stg_git=sockaddr_in
  +else
  +	ALL_CFLAGS += -Dsockaddr_stg_git=sockaddr_storage
>  endif
>  
>  ifdef PPC_SHA1
  But of course when this goes on, soon the gcc commandline will get
really awfully ugly. This is why something _like_ autoconf is a good
thing - you can just detect if the system headers provide the type and
#define it only when they don't.
  Of course, inserting the #define to some .h file might work; I'm not
sure if any of the relevant socket routines is permitted to be a macro
which could expand to something - but even then, it's probably not very
likely that it would break stuff in practice.
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Of the 3 great composers Mozart tells us what it's like to be human,
Beethoven tells us what it's like to be Beethoven and Bach tells us
what it's like to be the universe.  -- Douglas Adams
Alex Riesen· Jan 19, 2006, 15:07 UTC · re: Petr Baudis · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On 1/19/06, Petr Baudis <pasky@suse.cz> wrote:
>   But of course when this goes on, soon the gcc commandline will get
> really awfully ugly. This is why something _like_ autoconf is a good
> thing - you can just detect if the system headers provide the type and
> #define it only when they don't.

not you. Someone has done the job of detecting things for you. It's just a fair amount of hard work badly done.

Junio C Hamano· Jan 20, 2006, 01:13 UTC · re: Alex Riesen · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

Alex Riesen <raa.lkml@gmail.com> writes:
> For reasons unknown, cygwin decided to use our sockaddr_storage.
> As it is redefined to sockaddr_in it'd cause compilation errors in
> cygwin headers.

Just removing "-Dsockaddr_storage=sockaddr_in" from ALL_CFLAGS seems to solve the problem for new Cygwin.

I doubt that -Dsockaddr_storage=sockaddr_in should be tied to NO_IPV6. Maybe a better approach would be something like the attached patch.

Although /usr/include/cygwin/in.h seems to have struct sockaddr_in6, getaddrinfo() and friends still do not seem to be there, so NO_IPV6 still remains in effect on Cygwin.

Comments?

-- >8 -- [PATCH] Makefile: do not assume lack of IPV6 means no sockaddr_storage.

Noticed first by Alex, that the latest Cygwin now properly has sockaddr_storage.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 Makefile |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)
0b61a10bb258ae0971bce3e01e40a5a9e5d9537f
diff --git a/Makefile b/Makefile
index f6d9e0a..e8f4b24 100644
--- a/Makefile
+++ b/Makefile
@@ -42,6 +42,9 @@ all:
 #
 # Define NO_IPV6 if you lack IPv6 support and getaddrinfo().
 #
+# Define NO_SOCKADDR_STORAGE if your platform does not have struct
+# sockaddr_storage.
+#
 # Define COLLISION_CHECK below if you believe that SHA1's
 # 1461501637330902918203684832716283019655932542976 hashes do not give you
 # sufficient guarantee that no collisions between objects will ever happen.
@@ -342,7 +345,14 @@ ifdef NO_MMAP
 	COMPAT_OBJS += compat/mmap.o
 endif
 ifdef NO_IPV6
-	ALL_CFLAGS += -DNO_IPV6 -Dsockaddr_storage=sockaddr_in
+	ALL_CFLAGS += -DNO_IPV6
+endif
+ifdef NO_SOCKADDR_STORAGE
+ifdef NO_IPV6
+	ALL_CFLAGS += -Dsockaddr_storage=sockaddr_in
+else
+	ALL_CFLAGS += -Dsockaddr_storage=sockaddr_in6
+endif
 endif
 
 ifdef PPC_SHA1
-- 
1.1.3-gacdd
Alex Riesen· Jan 20, 2006, 13:23 UTC · re: Junio C Hamano · lore

Re: cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

On 1/20/06, Junio C Hamano <junkio@cox.net> wrote:
Show 12 quoted lines
> Just removing "-Dsockaddr_storage=sockaddr_in" from ALL_CFLAGS
> seems to solve the problem for new Cygwin.
>
> I doubt that -Dsockaddr_storage=sockaddr_in should be tied to
> NO_IPV6.  Maybe a better approach would be something like the
> attached patch.
>
> Although /usr/include/cygwin/in.h seems to have struct sockaddr_in6,
> getaddrinfo() and friends still do not seem to be there, so
> NO_IPV6 still remains in effect on Cygwin.
>
> Comments?
It compiles, I don't think I can try the daemon though...
Junio C Hamano· Jan 20, 2006, 01:13 UTC · re: Alex Riesen · lore

[PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

The recent Cygwin defines DT_UNKNOWN although it does not have d_type in struct dirent. Give an option to tell us not to use d_type on such platforms. Hopefully this problem will be transient.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 * By sending this out, I am not claiming this is better than
   your version or anything like that.  This is just for
   completeness and convenience, because my other two patches
   touch overlapping areas in the same Makefile.  This patch
   comes after the other two.
 Makefile |    7 +++++++
 cache.h  |    2 +-
 2 files changed, 8 insertions(+), 1 deletions(-)
80a4322a9bfbf0389ba8cb50f674682349f40a3a
diff --git a/Makefile b/Makefile
index 31898f7..84f5d24 100644
--- a/Makefile
+++ b/Makefile
@@ -21,6 +21,9 @@ all:
 #
 # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
 #
+# Define NO_D_TYPE_IN_DIRENT if your platform defines DT_UNKNOWN but lacks
+# d_type in struct dirent (latest Cygwin -- will be fixed soonish).
+#
 # Define NO_STRCASESTR if you don't have strcasestr.
 #
 # Define NO_SETENV if you don't have setenv in the C library.
@@ -233,6 +236,7 @@ ifeq ($(uname_S),SunOS)
 	ALL_CFLAGS += -D__EXTENSIONS__
 endif
 ifeq ($(uname_O),Cygwin)
+	NO_D_TYPE_IN_DIRENT = YesPlease
 	NO_D_INO_IN_DIRENT = YesPlease
 	NO_STRCASESTR = YesPlease
 	NEEDS_LIBICONV = YesPlease
@@ -335,6 +339,9 @@ ifdef NEEDS_NSL
 	LIBS += -lnsl
 	SIMPLE_LIB += -lnsl
 endif
+ifdef NO_D_TYPE_IN_DIRENT
+	ALL_CFLAGS += -DNO_D_TYPE_IN_DIRENT
+endif
 ifdef NO_D_INO_IN_DIRENT
 	ALL_CFLAGS += -DNO_D_INO_IN_DIRENT
 endif
diff --git a/cache.h b/cache.h
index 29c9e81..3d79c55 100644
--- a/cache.h
+++ b/cache.h
@@ -10,7 +10,7 @@
 #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)
 #endif
 
-#ifdef DT_UNKNOWN
+#if defined(DT_UNKNOWN) && !NO_D_TYPE_IN_DIRENT
 #define DTYPE(de)	((de)->d_type)
 #else
 #define DT_UNKNOWN	0
-- 
1.1.3-gacdd
Alex Riesen· Jan 20, 2006, 15:01 UTC · re: Junio C Hamano · lore

Re: [PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

On 1/20/06, Junio C Hamano <junkio@cox.net> wrote:
> The recent Cygwin defines DT_UNKNOWN although it does not have d_type
> in struct dirent.  Give an option to tell us not to use d_type on such
> platforms.  Hopefully this problem will be transient.

You still have to #undef all the DT_ macros if you have a somewhat old Cygwin (before Christopher removed the macros).

Junio C Hamano· Jan 20, 2006, 19:10 UTC · re: Alex Riesen · lore

Re: [PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

Alex Riesen <raa.lkml@gmail.com> writes:
Show 8 quoted lines
> On 1/20/06, Junio C Hamano <junkio@cox.net> wrote:
>> The recent Cygwin defines DT_UNKNOWN although it does not have d_type
>> in struct dirent.  Give an option to tell us not to use d_type on such
>> platforms.  Hopefully this problem will be transient.
>
> You still have to #undef all the DT_ macros if you have a somewhat old Cygwin
> (before Christopher removed the macros).
> -
Ah, you mean something like this?
-- >8 --
diff --git a/cache.h b/cache.h
index 8339931..6f13434 100644
--- a/cache.h
+++ b/cache.h
@@ -13,6 +13,10 @@
 #if defined(DT_UNKNOWN) && !NO_D_TYPE_IN_DIRENT
 #define DTYPE(de)	((de)->d_type)
 #else
+#undef DT_UNKNOWN
+#undef DT_DIR
+#undef DT_REG
+#undef DT_LNK
 #define DT_UNKNOWN	0
 #define DT_DIR		1
 #define DT_REG		2
Alex Riesen· Jan 20, 2006, 21:53 UTC · re: Junio C Hamano · lore

Re: [PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

Junio C Hamano, Fri, Jan 20, 2006 20:10:59 +0100:
Show 13 quoted lines
> >> The recent Cygwin defines DT_UNKNOWN although it does not have d_type
> >> in struct dirent.  Give an option to tell us not to use d_type on such
> >> platforms.  Hopefully this problem will be transient.
> >
> > You still have to #undef all the DT_ macros if you have a somewhat
> > old Cygwin (before Christopher removed the macros).
> 
> Ah, you mean something like this?
> 
> +#undef DT_UNKNOWN
> +#undef DT_DIR
> +#undef DT_REG
> +#undef DT_LNK
yes, of course
Junio C Hamano· Jan 21, 2006, 08:09 UTC · re: Alex Riesen · lore

Re: [PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

Alex Riesen <raa.lkml@gmail.com> writes:
Show 6 quoted lines
>> +#undef DT_UNKNOWN
>> +#undef DT_DIR
>> +#undef DT_REG
>> +#undef DT_LNK
>
> yes, of course

That is technically correct but I suspect it would not matter in practice. The only thing you are avoiding is four compiler warnings when compiling for Cygwin of this week (I heard this is already fixed in Cygwin CVS). On older Cygwin you simply would not use NO_D_TYPE_IN_DIRENT, since d_type worked before. No?

Alex Riesen· Jan 23, 2006, 13:07 UTC · re: Junio C Hamano · lore

Re: [PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

On 1/21/06, Junio C Hamano <junkio@cox.net> wrote:
Show 14 quoted lines
> Alex Riesen <raa.lkml@gmail.com> writes:
>
> >> +#undef DT_UNKNOWN
> >> +#undef DT_DIR
> >> +#undef DT_REG
> >> +#undef DT_LNK
> >
> > yes, of course
>
> That is technically correct but I suspect it would not matter in
> practice.  The only thing you are avoiding is four compiler
> warnings when compiling for Cygwin of this week (I heard this is
> already fixed in Cygwin CVS).  On older Cygwin you simply would
> not use NO_D_TYPE_IN_DIRENT, since d_type worked before.  No?
Yes :)

← back to recent threads