HP-UX allows directories to be opened with fopen (path, "r"), which

will cause some translations that expect to read files, read dirs

instead. This patch makes sure the two fopen () calls in remote.c

only open the file if it is a file.
Signed-off-by: H.Merijn Brand <h.m.brand@xs4all.nl>

---
diff -pur git-1.5.4a/remote.c git-1.5.4b/remote.c

--- git-1.5.4a/remote.c    2008-01-27 09:04:18 +0100

+++ git-1.5.4/remote.c     2008-02-08 17:38:43 +0100

@@ -121,9 +121,18 @@ static struct branch *make_branch(const

        return branches[empty];

 }
+/* Helper function to ensure that we are opening a file and not a directory */

+static FILE *open_file(char *full_path)

+{

+       struct stat st_buf;

+       if (stat(full_path, &st_buf) || !S_ISREG(st_buf.st_mode))

+               return NULL;

+       return (fopen(full_path, "r"));

+}

+

 static void read_remotes_file(struct remote *remote)

 {

-       FILE *f = fopen(git_path("remotes/%s", remote->name), "r");

+       FILE *f = open_file(git_path("remotes/%s", remote->name));
        if (!f)

                return;

@@ -173,7 +182,7 @@ static void read_branches_file(struct re

        char *frag;

        char *branch;

        int n = slash ? slash - remote->name : 1000;

-       FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r");

+       FILE *f = open_file(git_path("branches/%.*s", n, remote->name));

        char *s, *p;

        int len;
--

git-1.5.4
--

H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)

using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,

& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org

http://mirrors.develooper.com/hpux/            http://www.test-smoke.org

                        http://www.goldmark.org/jeff/stupid-disclaimers/

-

That looks wrong.  stat+fopen has a pointless race condition that

open+fstat+fdopen would not have.
Morten

-

That's true.  How about doing something like this?
 (1) in a new file "compat/gitfopen.c" have this:
	#include "../git-compat-util.h"

	#undef fopen

	FILE *gitfopen(const char *path, const char *mode)

        {

		int fd, flags;

                struct stat st;

        	if (mode[0] == 'w')

                	return fopen(path, mode);

		switch (mode[0]) {

                case 'r': flags = O_RDONLY; break;

                case 'a': flags = O_APPEND; break;

		default:

			errno = EINVAL;

                	return NULL;

		}

		fd = open(path, flags);

		if (fd < 0 || fstat(fd, &st))

                	return NULL;

		if (S_ISDIR(st_buf.st_mode)) {

                	errno = EISDIR;

                        return NULL;

		}

		return fdopen(fd, mode);

	}
  (2) in "git-compat-util.h" have this:
	#ifdef FOPEN_OPENS_DIRECTORIES

        #define fopen(a,b) gitfopen(a,b)

	extern FILE *gitfopen(const char *, const char *);

        #endif
And have Makefile set FOPEN_OPENS_DIRECTORIES on appropriate

platforms.

-

Can we use fileno()? Something like:
FILE *gitfopen(const char *path, const char *mode)

{

        FILE *fp;

        struct stat st;
        if (strpbrk(mode, "wa"))

                return fopen(path, mode);
        if (!(fp = fopen(path, mode)))

                return NULL;
        if (fstat(fileno(fp), &st)) {

                fclose(fp);

                return NULL;

        }
        if (S_ISDIR(st.st_mode)) {

                fclose(fp);

                errno = EISDIR;

                return NULL;

        }
        return fp;

}   
-brandon
-

Some systems do not fail as expected when fread et al. are called on

a directory stream. Replace fopen on such systems which will fail

when the supplied path is a directory.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>

---

 Makefile          |    7 +++++++

 compat/fopen.c    |   26 ++++++++++++++++++++++++++

 git-compat-util.h |    5 +++++

 3 files changed, 38 insertions(+), 0 deletions(-)

 create mode 100644 compat/fopen.c
diff --git a/Makefile b/Makefile

index 92341c4..debfc23 100644

--- a/Makefile

+++ b/Makefile

@@ -3,6 +3,9 @@ all::
 # Define V=1 to have a more verbose compile.

 #

+# Define FREAD_READS_DIRECTORIES if your are on a system which succeeds

+# when attempting to read from an fopen'ed directory.

+#

 # Define NO_OPENSSL environment variable if you do not have OpenSSL.

 # This also implies MOZILLA_SHA1.

 #

@@ -618,6 +621,10 @@ endif

 ifdef NO_C99_FORMAT

 	BASIC_CFLAGS += -DNO_C99_FORMAT

 endif

+ifdef FREAD_READS_DIRECTORIES

+	COMPAT_CFLAGS += -DFREAD_READS_DIRECTORIES

+	COMPAT_OBJS += compat/fopen.o

+endif

 ifdef NO_SYMLINK_HEAD

 	BASIC_CFLAGS += -DNO_SYMLINK_HEAD

 endif

diff --git a/compat/fopen.c b/compat/fopen.c

new file mode 100644

index 0000000..ccb9e89

--- /dev/null

+++ b/compat/fopen.c

@@ -0,0 +1,26 @@

+#include "../git-compat-util.h"

+#undef fopen

+FILE *git_fopen(const char *path, const char *mode)

+{

+	FILE *fp;

+	struct stat st;

+

+	if (mode[0] == 'w' || mode[0] == 'a')

+		return fopen(path, mode);

+

+	if (!(fp = fopen(path, mode)))

+		return NULL;

+

+	if (fstat(fileno(fp), &st)) {

+		fclose(fp);

+		return NULL;

+	}

+

+	if (S_ISDIR(st.st_mode)) {

+		fclose(fp);

+		errno = EISDIR;

+		return NULL;

+	}

+

+	return fp;

+}

diff --git a/git-compat-util.h b/git-compat-util.h

index 4df90cb..46d5e93 100644

--- a/git-compat-util.h

+++ b/git-compat-util.h

@@ -204,6 +204,11 @@ void *gitmemmem(const void *haystack, size_t haystacklen,

                 const void *needle, size_t needlelen);

 #endif
+#ifde...
[ message continues ]

On Fri, 08 Feb 2008 20:32:47 -0600, Brandon Casey <casey@nrlssc.navy.mil>
I applied this patch instead of mine, and added the Makefile define

Harder to trace, as it is not issuing error messages, but could this

success^Wfailure be related?
/pro/3gl/LINUX/git-1.5.4.rc5 103 > cat t/t5701-clone-local.sh.err

*   ok 1: preparing origin repository

*   ok 2: local clone without .git suffix

*   ok 3: local clone with .git suffix

*   ok 4: local clone from x

* FAIL 5: local clone from x.git that does not exist
                cd "$D" &&

                if git clone -l -s x.git z

                then

                        echo "Oops, should have failed"

                        false

                else

                        echo happy

                fi
*   ok 6: With -no-hardlinks, local will make a copy

*   ok 7: Even without -l, local will make a hardlink

* failed 1 among 7 test(s)
--

H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)

using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,

& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org

http://mirrors.develooper.com/hpux/            http://www.test-smoke.org

                        http://www.goldmark.org/jeff/stupid-disclaimers/

-

On Mon, 11 Feb 2008 10:29:50 +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl>
No, it is not. Some shell weirdness. This fixes it. Don't know off-hand

if it is portable enough
diff -pur a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh

--- a/t/t5701-clone-local.sh  2008-02-02 05:09:01 +0100

+++ b/t/t5701-clone-local.sh  2008-02-11 11:13:26 +0100

@@ -37,8 +37,8 @@ test_expect_success 'local clone from x'
 test_expect_success 'local clone from x.git that does not exist' '

        cd "$D" &&

-       if git clone -l -s x.git z

-       then

+       git clone -l -s x.git z

+       if $? ; then

                echo "Oops, should have failed"

                false

        else
--

H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)

using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,

& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org

http://mirrors.develooper.com/hpux/            http://www.test-smoke.org

                        http://www.goldmark.org/jeff/stupid-disclaimers/

-

I think your "git clone" is broken and I strongly suspect it is

not your shell (at least the "if" construct in the test).
What's 
	if $?; then
In sane shells, I think this tries to execute 0 or perhaps 124

or whatever the error code from clone as if it was the name of a

command, which would most likely fail and would not take "then"

part (which reports the error).  It did not fix, but just made

it ignore the error from "git clone".
If it were
	if test $? != 0

        then
it would have made a bit more sense.
And if (this is a big "if" as I doubt any shell is so broken)

these two are equivalent to your shell, then I do not think it

is portable at all.

-

of course it should have been 'if test $?' and as $? is erroneously

equal to 0 in this case, this snippet doesn't matter
'git clone' is calling 'cit-clone' which is a shell script, that does

exit 1 in the function die:

--8<---

die() {

	echo >&2 "$@"

	exit 1

}

-->8---
--

H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)

using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,

& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org

http://mirrors.develooper.com/hpux/            http://www.test-smoke.org

                        http://www.goldmark.org/jeff/stupid-disclaimers/

-

Which ones _don't_ open directories?
Shouldn't fopen("path_to_some_directory", "r") work?
-brandon
#include <stdio.h>
int main(int argc, char* argv[])

{

    if (!fopen(argv[1], "r")) {

        perror("File open failed");

        return 1;

    }
    puts("File open succeeded.");
    return 0;

}
-

Ahh, sorry, of course you are right.  We need to fix the

callers.
-

Ok. It's the FOPEN_OPENS_DIRECTORIES term that confused me.
fopen is expected to succeed, even on directories. It's the read

that should fail but is not on HPUX. Obviously we don't want to

change the read.
-brandon
-

Hi,
Funny... our emails crossed, and you picked the same name ;-)
Ciao,

Dscho
-

Bad Dscho.
It has been a very well kept secret that Dscho and Junio are one

and the same person, but you just spilled the beans.
;-)

-

Hi,
Shush... left-hemisphere: shut up.
Ciao,

Dscho
P.S.: double ;-)
-

Can we make this a platform specific "compat" hack?
It is not fair to force stat() overhead to ports on platforms

that fails fopen() on directories, as I doubt we would ever want

from directory using fopen() anyway.

-

The two I patched were in remote.c and do not happen on every file if I

analyzed it correctly, so overhead would be minimal. However, as I read

the rest of the discussion already, your approach to fix all fopen ()

calls at once seems very reasonable.
I didn't check
--

H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)

using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,

& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org

http://mirrors.develooper.com/hpux/            http://www.test-smoke.org

                        http://www.goldmark.org/jeff/stupid-disclaimers/

-

Hi,
You mean something like
#ifdef FOPEN_OPENS_DIRECTORIES

inline static FILE *fopen_compat(const char *path, const char *mode)

{

       struct stat st_buf;

       if (stat(path, &st_buf) || !S_ISREG(st_buf.st_mode))

               return NULL;

       return (fopen(path, mode));

}

#define fopen fopen_compat

#endif
in git-compat-util.h, right?
Yeah, I can see that, even if I think the overhead would not be _that_

crucial.  But it is a nice way of fixing _all_ fopen() calls at the same

time.
Ciao,

Dscho

-

Many thanks, this is also required for AIX. I had got some way to

tracking it down, but I thought it was an issue with strbuf. So...
Tested-by: Mike Ralphson <mike.ralphson@gmail.com>
Your other fix there [- if (!strbuf_avail(sb)) / + if

(strbuf_avail(sb) < 64) ] is, guess what, also required on AIX.
Thanks again.

-

Does the following help? We really ought to know that ".." must be a path

literal (and there obviously should be more limitations on nicknames for

remotes, but I haven't figured out what they should be yet).
	-Daniel

*This .sig left intentionally blank*
diff --git a/remote.c b/remote.c

index 0e00680..83a3d9d 100644

--- a/remote.c

+++ b/remote.c

@@ -348,7 +348,7 @@ struct remote *remote_get(const char *name)

        if (!name)

                name = default_remote_name;

        ret = make_remote(name, 0);

-       if (name[0] != '/') {

+       if (name[0] != '/' && strcmp(name, "..")) {

                if (!ret->url)

                        read_remotes_file(ret);

                if (!ret->url)
-

Perhaps "static int valid_remote_nick(const char*)" is needed?

I'd say we can limit it to something like:
static int valid_remote_nick(const char *name)

{

	if (!name[0] || /* not empty */

            (name[0] == '.' && /* not "." */

             (!name[1] || /* not ".." */

              (name[1] == '.' && !name[2]))))

		return 0;

	return !!strchr(name, '/'); /* no slash */

}
-

Yeah, that looks right to me.
	-Daniel

*This .sig left intentionally blank*

-

Hi,
You'll need to check for ".", too: "git pull . <branch>" was originally

the only way to merge a local branch, and it is still valid.
Ciao,

Dscho
-

On Fri, 8 Feb 2008 17:25:52 +0000, "Mike Ralphson" <mike.ralphson@gmail.com>
Not there yet ...
$ cat do-tests

#!/bin/sh
export TAR=ntar

rm -f *.err

for t in t[0-9]*.sh ; do

    echo $t

    sh $t > test.err 2>&1 || mv test.err $t.err

    rm -f test.err

    done

$
197509 -rw-rw-rw- 1 merijn softwr 1633 Feb  8 18:03 t5302-pack-index.sh.err

196846 -rw-rw-rw- 1 merijn softwr  943 Feb  8 18:04 t5500-fetch-pack.sh.err

203431 -rw-rw-rw- 1 merijn softwr  344 Feb  8 18:05 t5600-clone-fail-cleanup.sh.err

202602 -rw-rw-rw- 1 merijn softwr  458 Feb  8 18:05 t5701-clone-local.sh.err

202761 -rw-rw-rw- 1 merijn softwr 3039 Feb  8 18:06 t6002-rev-list-bisect.sh.err

202641 -rw-rw-rw- 1 merijn softwr 3980 Feb  8 18:06 t6003-rev-list-topo-order.sh.err

202731 -rw-rw-rw- 1 merijn softwr  899 Feb  8 18:06 t6022-merge-rename.sh.err

197510 -rw-rw-rw- 1 merijn softwr 1340 Feb  8 18:08 t7201-co.sh.err

202705 -rw-rw-rw- 1 merijn softwr  149 Feb  8 18:09 t9300-fast-import.sh.err

197051 -rw-rw-rw- 1 merijn softwr 1651 Feb  8 18:09 t9301-fast-export.sh.err
http://www.xs4all.nl/~procura/git-1.5.3-1123ipf.tar
Tips welcome :)
--

H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)

using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,

& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org

http://mirrors.develooper.com/hpux/            http://www.test-smoke.org

                        http://www.goldmark.org/jeff/stupid-disclaimers/

-

On Fri, 8 Feb 2008 21:04:47 +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl>
Most bizarre workaround found for clone (the first 4 failures):

--8<---

diff -pur /a5/pro/3gl/LINUX/git-1.5.4/git-clone.sh git-clone.sh

--- a/git-1.5.4/git-clone.sh  2008-02-02 05:09:01 +0100

+++ b/git-1.5.4/git-clone.sh  2008-02-18 10:03:26 +0100

@@ -368,7 +368,8 @@ yes)

                '') git-fetch-pack --all -k $quiet $depth $no_progress "$repo";;

                *) git-fetch-pack --all -k $quiet "$upload_pack" $depth $no_progress "$repo" ;;

                esac >"$GIT_DIR/CLONE_HEAD" ||

-                       die "fetch-pack from '$repo' failed."

+                       exit 1

+                       # die "fetch-pack from '$repo' failed."

                ;;

        esac

        ;;

-->8---
4 down, 6 to go

197225 -rw-rw-rw- 1 merijn softwr  7246 Feb 18 09:58 t6002-rev-list-bisect.sh.err

197111 -rw-rw-rw- 1 merijn softwr 10763 Feb 18 09:58 t6003-rev-list-topo-order.sh.err

197190 -rw-rw-rw- 1 merijn softwr 17903 Feb 18 09:58 t6022-merge-rename.sh.err

196841 -rw-rw-rw- 1 merijn softwr  9299 Feb 18 10:00 t7201-co.sh.err

196928 -rw-rw-rw- 1 merijn softwr   484 Feb 18 10:01 t9300-fast-import.sh.err

196683 -rw-rw-rw- 1 merijn softwr  5035 Feb 18 10:01 t9301-fast-export.sh.err
--

H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)

using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,

& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org

http://mirrors.develooper.com/hpux/            http://www.test-smoke.org

                        http://www.goldmark.org/jeff/stupid-disclaimers/

-

That sounds *very* broken.
Is your /bin/sh really a variant of Bourne?
If HP-UX is broken in a similar way as Solaris is, in that it

installs a non-POSIX shell under /bin/sh and offers a Korn in

/bin/ksh, "make SHELL_PATH=/bin/ksh" may help.

-

Indeed, and trying to see if eval or exiting from with a sub caused

this weird behaviour, I failed to come up with a simple test script
Yes
 NAME

      sh - overview of various system shells
 SYNOPSIS

    POSIX Shell:

      sh [+-aefhikmnoprstuvx] [+-o option] ...  [-c string] [arg ...]
      rsh [+-aefhikmnoprstuvx] [+-o option] ...  [-c string] [arg ...]
    Korn Shell:

      ksh [+-aefhikmnoprstuvx] [+-o option] ...  [-c string] [arg ...]
      rksh [+-aefhikmnoprstuvx] [+-o option] ...  [-c string] [arg ...]
    C Shell:

      csh [-cefinstvxTVX] [command_file] [argument_list ...]
    Key Shell:
$ path -al sh ksh

  27231 100555 -r-x    2      bin    586136  27 Aug 2004 03:36 /usr/bin/sh

   1744 100555 -r-x    1      bin   1219780  27 Aug 2004 03:36 /sbin/sh

   3206 100555 -r-x    2      bin    446904  27 Aug 2004 03:20 /usr/bin/ksh
And running all with ksh only makes things worse!
$ cat t0000-basic.sh.err

t0000-basic.sh[31]: !:  not found

test_expect_success[31]: !:  not found

test_expect_success[31]: !:  not found

test_expect_failure[31]: !:  not found

test_expect_success[31]: !:  not found

test_expect_success[31]: !:  not found

test_expect_success[31]: !:  not found
--

H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)

using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,

& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org

http://mirrors.develooper.com/hpux/            http://www.test-smoke.org

                        http://www.goldmark.org/jeff/stupid-disclaimers/

-