Git development
 help / color / mirror / Atom feed
* [PATCH Cogito] cg-fetch: fix local cloning with symbolic refs
From: Jonas Fonseca @ 2005-10-08 17:48 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git

Ugly workaround for making the HEAD getter use 'git-symbolic-ref HEAD'
so that git-local-fetch is passed the proper ID and not 'ref: ...'.

---

Yeah, it is ugly, it assumes we are getting the HEAD (which is currently
the only one using the -b flag.

diff --git a/cg-fetch b/cg-fetch
index d0d37e1..57096cd 100755
--- a/cg-fetch
+++ b/cg-fetch
@@ -248,9 +248,10 @@ fetch_ssh()
 
 get_local()
 {
+	symref=
 	cp_flags_l="-vdpR"
 	if [ "$1" = "-b" ]; then
-		cp_flags_l="-vb" # Dereference symlinks
+		symref=1
 		shift
 	fi
 
@@ -270,6 +271,7 @@ get_local()
 
 	src="$1"
 	dest="$2"
+	[ "$symref" ] && src="$(dirname $src)/$(git-symbolic-ref HEAD)"
 	[ "$cut_last" ] && dest=${dest%/*}
 
 	cp $cp_flags_l "$src" "$dest"

-- 
Jonas Fonseca

^ permalink raw reply related

* Re: First cut at git port to Cygwin
From: Elfyn McBratney @ 2005-10-08 17:43 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0510080900510.31407@g5.osdl.org>

[-- Attachment #1: Type: text/plain, Size: 424 bytes --]

Er, apologies for the dups - postfix crapped itself :/

*goes and stands in the corner donning the 'D' hat*

-- 
Elfyn McBratney
Gentoo Developer/Perl Team Lead
beu/irc.freenode.net                            http://dev.gentoo.org/~beu/
+------------O.o--------------------- http://dev.gentoo.org/~beu/pubkey.asc

PGP Key ID: 0x69DF17AD
PGP Key Fingerprint:
  DBD3 B756 ED58 B1B4 47B9  B3BD 8D41 E597 69DF 17AD

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH Cogito] Allow spaces in $HOME
From: Jonas Fonseca @ 2005-10-08 17:41 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git

Fixes sourcing of ~/.cgrc

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>

---

diff --git a/cg-Xlib b/cg-Xlib
index b27e8b9..1644913 100755
--- a/cg-Xlib
+++ b/cg-Xlib
@@ -336,8 +336,8 @@ ARGPOS=0
 
 if [ -t 1 -a -e "$HOME/.cgrc" ]; then
 	_cg_name=${_cg_cmd#cg-}
-	_cg_defaults1="$(sed -n "/^$_cg_cmd/s/^$_cg_cmd //p" < $HOME/.cgrc)"
-	_cg_defaults2="$(sed -n "/^$_cg_name/s/^$_cg_name //p" < $HOME/.cgrc)"
+	_cg_defaults1="$(sed -n "/^$_cg_cmd/s/^$_cg_cmd //p" < "$HOME/.cgrc")"
+	_cg_defaults2="$(sed -n "/^$_cg_name/s/^$_cg_name //p" < "$HOME/.cgrc")"
 	ARGS=($_cg_defaults1 $_cg_defaults2 "${ARGS[@]}")
 fi
 
-- 
Jonas Fonseca

^ permalink raw reply related

* Re: First cut at git port to Cygwin
From: Elfyn McBratney @ 2005-10-08 17:38 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Linus Torvalds, Alex Riesen, Chuck Lever, Git Mailing List,
	Junio C Hamano, Christopher Faylor, H. Peter Anvin
In-Reply-To: <Pine.LNX.4.64.0510080900510.31407@g5.osdl.org>

[-- Attachment #1: Type: text/plain, Size: 2779 bytes --]

On Sat, Oct 08, 2005 at 09:11:03AM -0700, Linus Torvalds wrote:
 > 
 > On Fri, 7 Oct 2005, Alex Riesen wrote:
 > > 
 > > Make read_cache copy the index into memory, to improve portability on
 > > other OS's which have mmap too, tend to use it less commonly.
 > 
 > I really think that you should just get rid of the mmap.
 > 
 > As it is, you're just slowing the code down on sane architectures. That's 
 > not good.
 > 
 > So I'd suggest something like this instead.
 > 
 > Totally untested, of course.
 > 
 > 		Linus

Slightly adjusted diff below so it compiles ;)  (Note: only the second
die() un hunk #1 was changed.)

Best,
Elfyn

----
diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ -454,13 +454,39 @@ static int verify_hdr(struct cache_heade
 	return 0;
 }
 
+static void *map_index_file(int fd, size_t size)
+{
+	void *map;
+#ifdef NO_MMAP
+	map = malloc(size);
+	if (!map)
+		die("Unable to allocate index file mapping");
+	if (read(fd, map, size) != size)
+		die("Unable to read %z bytes from index", size);
+#else
+	map = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+	if (map == MAP_FAILED)
+		die("index file mmap failed (%s)", strerror(errno));
+#endif
+	return map;
+}
+
+static void unmap_index_file(void *map, size_t size)
+{
+#ifdef NO_MMAP
+	free(map);
+#else
+	munmap(map, size);
+#endif
+}
+
 int read_cache(void)
 {
 	int fd, i;
 	struct stat st;
 	unsigned long size, offset;
-	void *map;
 	struct cache_header *hdr;
+	void *map;
 
 	errno = EBUSY;
 	if (active_cache)
@@ -475,16 +501,15 @@ int read_cache(void)
 	}
 
 	size = 0; // avoid gcc warning
-	map = MAP_FAILED;
-	if (!fstat(fd, &st)) {
-		size = st.st_size;
-		errno = EINVAL;
-		if (size >= sizeof(struct cache_header) + 20)
-			map = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
-	}
+	if (fstat(fd, &st))
+		die("unable to fstat index file");
+
+	size = st.st_size;
+	errno = EINVAL;
+	if (size < sizeof(struct cache_header) + 20)
+		goto corrupt;
+	map = map_index_file(fd, size);
 	close(fd);
-	if (map == MAP_FAILED)
-		die("index file mmap failed (%s)", strerror(errno));
 
 	hdr = map;
 	if (verify_hdr(hdr, size) < 0)
@@ -503,8 +528,9 @@ int read_cache(void)
 	return active_nr;
 
 unmap:
-	munmap(map, size);
+	unmap_index_file(map, size);
 	errno = EINVAL;
+corrupt:
 	die("index file corrupt");
 }


-- 
Elfyn McBratney
Gentoo Developer/Perl Team Lead
beu/irc.freenode.net                            http://dev.gentoo.org/~beu/
+------------O.o--------------------- http://dev.gentoo.org/~beu/pubkey.asc

PGP Key ID: 0x69DF17AD
PGP Key Fingerprint:
  DBD3 B756 ED58 B1B4 47B9  B3BD 8D41 E597 69DF 17AD

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH Cogito] Use git-{update,symbolic}-ref for updating HEAD
From: Jonas Fonseca @ 2005-10-08 16:57 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20051002101224.GA9219@diku.dk>

Conditionally make git-update-ref check the old head. For this to succeed
for the initial commit, cg-init should not touch .git/refs/heads/master,
and the touching seems to be redundant anyway, so remove it.

This should make Cogito mostly work on cygwin.

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>

---

This is an incremental update to make Cogito handle symbolic refs.

 I am not sure whether the change from using '[ -L .git/HEAD ]' to using
'[ -e .git/HEAD ]' is the thing to do. And I haven't used it so much
yet; did a few commit and so on. And the tests fails do to local cloning
not working yet (git-local-fetch needs to be fixed).

 Makefile                        |    2 +-
 cg-Xlib                         |    4 ++--
 cg-commit                       |    4 ++--
 cg-init                         |    1 -
 cg-object-id                    |    2 +-

---

diff --git a/Makefile b/Makefile
index f0a0590..9eacade 100644
--- a/Makefile
+++ b/Makefile
@@ -35,7 +35,7 @@ all: cogito
 cogito: $(GEN_SCRIPT)
 
 ifneq (,$(wildcard .git))
-GIT_HEAD=.git/HEAD
+GIT_HEAD=.git/$(shell git-symbolic-ref HEAD)
 GIT_HEAD_ID=" \($(shell cat $(GIT_HEAD))\)"
 endif
 cg-version: $(VERSION) $(GIT_HEAD) Makefile
diff --git a/cg-Xlib b/cg-Xlib
index b27e8b9..dcf9aa3 100755
--- a/cg-Xlib
+++ b/cg-Xlib
@@ -246,7 +246,7 @@ tree_timewarp()
 	fi
 
 	git-read-tree -m "$branch" || die "$branch: bad commit"
-	[ "$no_head_update" ] || echo "$branch" > $_git/HEAD
+	[ "$no_head_update" ] || git-update-ref HEAD "$branch"
 
 	# Kill gone files
 	git-diff-tree -z -r $base $branch | xargs -0 bash -c '
@@ -471,7 +471,7 @@ if [ ! "$_git_repo_unneeded" ]; then
 	       exit 1
 	fi
 	_git_head=master
-	[ -L "$_git/HEAD" ] && _git_head="$(basename "$(readlink "$_git/HEAD")")"
+	[ -e "$_git/HEAD" ] && _git_head="$(basename "$(git-symbolic-ref HEAD)")"
 	[ -s "$_git/head-name" ] && _git_head="$(cat "$_git/head-name")"
 fi
 
diff --git a/cg-commit b/cg-commit
index 4345bd5..6024f17 100755
--- a/cg-commit
+++ b/cg-commit
@@ -396,7 +396,7 @@ fi
 
 oldhead=
 if [ -s "$_git/HEAD" ]; then
-	oldhead=$(cat $_git/HEAD)
+	oldhead=$(git-symbolic-ref HEAD)
 	oldheadstr="-p $oldhead"
 fi
 
@@ -420,7 +420,7 @@ fi
 
 if [ "$newhead" ]; then
 	echo "Committed as $newhead."
-	echo $newhead >$_git/HEAD
+	git-update-ref HEAD $newhead $oldhead 
 	[ "$merging" ] && rm $_git/merging $_git/merging-sym $_git/merge-base
 	rm -f "$_git/commit-ignore"
 
diff --git a/cg-init b/cg-init
index 570c83e..65faf84 100755
--- a/cg-init
+++ b/cg-init
@@ -48,7 +48,6 @@ done
 cleanup_trap "rm -rf $_git"
 
 git-init-db
-touch $_git/refs/heads/master
 
 git-read-tree # Seed the dircache
 if ! [ "$no_initial_commit" ]; then
diff --git a/cg-object-id b/cg-object-id
index 2ae9420..5c03626 100755
--- a/cg-object-id
+++ b/cg-object-id
@@ -53,7 +53,7 @@ normalize_id()
 	fi
 
 	if [ ! "$id" ] || [ "$id" = "this" ] || [ "$id" = "HEAD" ]; then
-		read id < "$_git/HEAD"
+		read id < "$_git/$(git-symbolic-ref HEAD)"
 
 	elif [ -r "$_git/refs/tags/$id" ]; then
 		read id < "$_git/refs/tags/$id"
-- 
Jonas Fonseca

^ permalink raw reply related

* [PATCH] Restore functionality to allow proxies to cache objects
From: Nick Hengeveld @ 2005-10-08 16:40 UTC (permalink / raw)
  To: git

The parallel request changes didn't properly implement the previous patch to
allow caching of retrieved objects by proxy servers.  Restore the previous
functionality such that by default requests include the "Pragma: no-cache"
header, and this header is removed on requests for pack indexes, packs, and
objects.

Signed-off-by: Nick Hengeveld <nickh@reactrix.com>


---

Reading the revision history is useful...


 http-fetch.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

6a9799a063e53764338d8c54b17464f46321ec60
diff --git a/http-fetch.c b/http-fetch.c
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -29,6 +29,7 @@ static int max_requests = DEFAULT_MAX_RE
 static CURLM *curlm;
 #endif
 static CURL *curl_default;
+static struct curl_slist *pragma_header;
 static struct curl_slist *no_pragma_header;
 static struct curl_slist *no_range_header;
 static char curl_errorstr[CURL_ERROR_SIZE];
@@ -203,7 +204,7 @@ struct active_request_slot *get_active_s
 	slot->in_use = 1;
 	slot->done = 0;
 	slot->local = NULL;
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, pragma_header);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_range_header);
 	curl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, curl_errorstr);
 
@@ -358,6 +359,7 @@ void start_request(struct transfer_reque
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);
 	curl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, request->errorstr);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header);
 
 	/* If we have successfully processed data from a previous fetch
 	   attempt, only fetch the data we don't already have. */
@@ -568,6 +570,7 @@ static int fetch_index(struct alt_base *
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, indexfile);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header);
 	slot->local = indexfile;
 
 	/* If there is data present from a previous transfer attempt,
@@ -837,6 +840,7 @@ static int fetch_pack(struct alt_base *r
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, packfile);
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite);
 	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header);
 	slot->local = packfile;
 
 	/* If there is data present from a previous transfer attempt,
@@ -1067,6 +1071,7 @@ int main(int argc, char **argv)
 		return 1;
 	}
 #endif
+	pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache");
 	no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:");
 	no_range_header = curl_slist_append(no_range_header, "Range:");
 
@@ -1106,6 +1111,7 @@ int main(int argc, char **argv)
 	if (pull(commit_id))
 		return 1;
 
+	curl_slist_free_all(pragma_header);
 	curl_slist_free_all(no_pragma_header);
 	curl_slist_free_all(no_range_header);
 	curl_easy_cleanup(curl_default);

^ permalink raw reply

* Re: First cut at git port to Cygwin
From: Linus Torvalds @ 2005-10-08 16:11 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Chuck Lever, Git Mailing List, Junio C Hamano, Christopher Faylor,
	H. Peter Anvin
In-Reply-To: <20051007213952.GA8821@steel.home>



On Fri, 7 Oct 2005, Alex Riesen wrote:
> 
> Make read_cache copy the index into memory, to improve portability on
> other OS's which have mmap too, tend to use it less commonly.

I really think that you should just get rid of the mmap.

As it is, you're just slowing the code down on sane architectures. That's 
not good.

So I'd suggest something like this instead.

Totally untested, of course.

		Linus

----
diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ -454,13 +454,39 @@ static int verify_hdr(struct cache_heade
 	return 0;
 }
 
+static void *map_index_file(int fd, size_t size)
+{
+	void *map;
+#ifdef NO_MMAP
+	map = malloc(size);
+	if (!map)
+		die("Unable to allocate index file mapping");
+	if (read(fd, map, size) != size)
+		die("Unable to read %z bytes from inde
+#else
+	map = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+	if (map == MAP_FAILED)
+		die("index file mmap failed (%s)", strerror(errno));
+#endif
+	return map;
+}
+
+static void unmap_index_file(void *map, size_t size)
+{
+#ifdef NO_MMAP
+	free(map);
+#else
+	munmap(map, size);
+#endif
+}
+
 int read_cache(void)
 {
 	int fd, i;
 	struct stat st;
 	unsigned long size, offset;
-	void *map;
 	struct cache_header *hdr;
+	void *map;
 
 	errno = EBUSY;
 	if (active_cache)
@@ -475,16 +501,15 @@ int read_cache(void)
 	}
 
 	size = 0; // avoid gcc warning
-	map = MAP_FAILED;
-	if (!fstat(fd, &st)) {
-		size = st.st_size;
-		errno = EINVAL;
-		if (size >= sizeof(struct cache_header) + 20)
-			map = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
-	}
+	if (fstat(fd, &st))
+		die("unable to fstat index file");
+
+	size = st.st_size;
+	errno = EINVAL;
+	if (size < sizeof(struct cache_header) + 20)
+		goto corrupt;
+	map = map_index_file(fd, size);
 	close(fd);
-	if (map == MAP_FAILED)
-		die("index file mmap failed (%s)", strerror(errno));
 
 	hdr = map;
 	if (verify_hdr(hdr, size) < 0)
@@ -503,8 +528,9 @@ int read_cache(void)
 	return active_nr;
 
 unmap:
-	munmap(map, size);
+	unmap_index_file(map, size);
 	errno = EINVAL;
+corrupt:
 	die("index file corrupt");
 }
 

^ permalink raw reply

* [PATCH] Try URI quoting for embedded TAB and LF in pathnames
From: Robert Fitzsimons @ 2005-10-08 13:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Riesen, git, Kai Ruemmler
In-Reply-To: <7vachks7aq.fsf@assigned-by-dhcp.cox.net>

Instead of using //{LF}// and //{TAG}// to quote embedded tab and
linefeed characters in pathnames use URI quoting.

'\t' becomes %09
'\n' becomes %10
'%' becomes %25

Signed-off-by: Robert Fitzsimons <robfitz@273k.net>

---

> I am not married to this quoting syntax -- I think it *is* ugly,
> but as I said before, I'd prefer to have something ugly here.
> 
> I would easily be persuaded otherwise, though.  A working patch
> would probably be the most effective way of persuasion, but a
> mock output without the code to produce and/or parse it would
> also be fine as a starting point for discussion.

Using URI encoding might be an option it's not a ugly and more peopel
should under stand what it means.  Heres a posible patch against pu.

Robert


 apply.c       |   19 ++++++++++++-------
 diff.c        |   26 +++++++++++++++++---------
 git-status.sh |   10 ++++++----
 3 files changed, 35 insertions(+), 20 deletions(-)

applies-to: a9332b0c2bd80a182f946d22d4ec7511c32c55f4
8029a957cab1a912562696fdce8beea5fc2c11c4
diff --git a/apply.c b/apply.c
--- a/apply.c
+++ b/apply.c
@@ -75,21 +75,26 @@ static char *unmunge_name(char *name)
 
 	if (!name)
 		return name;
-	cp = strstr(name, "//");
+	cp = strstr(name, "%");
 	if (!cp)
 		return name;
 	ret_name = strdup(name);
 	for (cp = dp = ret_name; (ch = *cp); cp++) {
-		if (ch == '/' && cp[1] == '/' && cp[2] == '{') {
-			/* //{TAB}// or //{LF}// */
-			if (!strncmp(cp + 3, "TAB}//", 6)) {
+		if (ch == '%') {
+			/* %09 or %10 or %25 */
+			if (!strncmp(cp + 1, "09", 2)) {
 				*dp++ = '\t';
-				cp += 8;
+				cp += 2;
 				continue;
 			}
-			else if (!strncmp(cp + 3, "LF}//", 5)) {
+			else if (!strncmp(cp + 1, "10", 2)) {
 				*dp++ = '\n';
-				cp += 7;
+				cp += 2;
+				continue;
+			}
+			else if (!strncmp(cp + 1, "25", 2)) {
+				*dp++ = '%';
+				cp += 2;
 				continue;
 			}
 			error("malformed munged name '%s' (looking at %s)",
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -13,7 +13,7 @@ static const char *path_munge(const char
 {
 	const char *cp;
 	char *retpath, *dp;
-	int ch, munge_inter_name = 0, munge_line_term = 0;
+	int ch, munge_inter_name = 0, munge_line_term = 0, munge_quote = 0;
 
 	if (!path)
 		return path;
@@ -23,23 +23,31 @@ static const char *path_munge(const char
 			munge_inter_name++;
 		if (line_term && ch == '\n')
 			munge_line_term++;
+		if (ch == '%')
+			munge_quote++;
 	}
-	if (!(munge_inter_name + munge_line_term))
+	if (!(munge_inter_name + munge_line_term + munge_quote))
 		return path;
 
-	/* need //{TAB}// and //{LF}// */
+	/* need %09 and %10 and %25 */
 	retpath = xmalloc(cp - path +
-			  munge_inter_name * 8 +
-			  munge_line_term * 7 + 1);
+			  munge_inter_name * 3 +
+			  munge_line_term * 3 +
+			  munge_quote * 3 + 1);
 	for (cp = path, dp = retpath; (ch = *cp); cp++, dp++) {
 		if (inter_name && ch == '\t') {
-			memcpy(dp, "//{TAB}//", 9);
-			dp += 8;
+			memcpy(dp, "%09", 3);
+			dp += 2;
 			continue;
 		}
 		if (line_term && ch == '\n') {
-			memcpy(dp, "//{LF}//", 8);
-			dp += 7;
+			memcpy(dp, "%10", 3);
+			dp += 2;
+			continue;
+		}
+		if (ch == '%') {
+			memcpy(dp, "%25", 3);
+			dp += 2;
 			continue;
 		}
 		*dp = ch;
diff --git a/git-status.sh b/git-status.sh
--- a/git-status.sh
+++ b/git-status.sh
@@ -54,8 +54,9 @@ else
 	perl -e '$/ = "\0";
 		while (<>) {
 			chomp;
-			s|\t|//{TAB}//|g;
-			s|\n|//{LF}//|g;
+			s|%([^021][^059])|%25\1|g;
+			s|\t|%09|g;
+			s|\n|%10|g;
 			s/ /\\ /g;
 			s/^/A /;
 			print "$_\n";
@@ -84,8 +85,9 @@ perl -e '$/ = "\0";
 	my $shown = 0;
 	while (<>) {
 		chomp;
-		s|\t|//{TAB}//|g;
-		s|\n|//{LF}//|g;
+		s|%([^01][^09])|%25\1|g;
+		s|\t|%09|g;
+		s|\n|%10|g;
 		s/^/#	/;
 		if (!$shown) {
 			print "#\n# Ignored files:\n";
---
0.99.8.GIT

^ permalink raw reply

* Re: [RFC] embedded TAB and LF in pathnames
From: Junio C Hamano @ 2005-10-08  9:10 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Kai Ruemmler
In-Reply-To: <20051008064555.GA3831@steel.home>

Alex Riesen <raa.lkml@gmail.com> writes:

          Quote  nongraphic  characters in file names using alphabetic and
          octal backslash sequences like those used in C. This  option  is
          the  same as -Q except that filenames are not surrounded by dou-
          ble-quotes.

If you have a file whose name is 'foo' + LF + 'bar', and if you
use backslash convention, your diff would start like this:

    diff --git a/foo\nbar b/foo\nbar
    @@ 1,2 3,4 @@
     context
    -deleted
    ...

which looks quite natural.

I would, however, prefer this kind of funny pathnames to *stand*
*out* more than usual, to make it really obvious that there is
something really funky going on.  In that sense, the above is a
bit too innocuous-looking to my taste.

But this "embedded LF and TAB" is a corner case.  I would not be
using such paths that would trigger the quoting myself anyway,
and I do not particularly care as long as the tools do the right
thing -- any quoting rule would do, as long as the generating
side (git-diff) is consistent with accepting side (git-apply),
and as long as there is no new ambiguity introduced.

The backslash proposal is introducing a small ambiguity.  You
cannot tell if the file had an embedded LF between 'foo' and
'bar' (and generated with your git-diff) or had an embedded
backslash between 'foo' and 'nbar' (and generated with existing
git-diff).  Since we never had a version of git-diff that
outputs double-slashes '//' in paths, there is no ambiguity if
we use it as a quoting mechanism.

Just as a concrete demonstration, here is how the git-status
output and git-diff output would look like for a file 'pqr' in a
directory whose name is 'def' + LF + 'ghi' that uses the version
of git-diff from the proposed updates branch:

        # Changed but not updated:
        #   (use git-update-index to mark for commit)
        #
        #	modified: def//{LF}//ghi/pqr

        diff --git a/def//{LF}//ghi/pqr b/def//{LF}//ghi/pqr
        index 9ee055c..47dbc3f 100644
        --- a/def//{LF}//ghi/pqr
        +++ b/def//{LF}//ghi/pqr
        @@ -1 +1,2 @@
         Fri Oct  7 23:19:04 PDT 2005
        +foo

I am not married to this quoting syntax -- I think it *is* ugly,
but as I said before, I'd prefer to have something ugly here.

I would easily be persuaded otherwise, though.  A working patch
would probably be the most effective way of persuasion, but a
mock output without the code to produce and/or parse it would
also be fine as a starting point for discussion.

^ permalink raw reply

* Re: [RFC] embedded TAB and LF in pathnames
From: Alex Riesen @ 2005-10-08  6:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Kai Ruemmler
In-Reply-To: <7vpsqgyjrj.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano, Sat, Oct 08, 2005 01:44:48 +0200:
> > Junio C Hamano, Fri, Oct 07, 2005 21:35:19 +0200:
> >> I have not made up my mind on the exact choice of the quoting
> >> convention.  We could say '///' instead of '//', for example, or
> >> even '//{LF}//' instead of '//0A' proposed above.  One thing I
> >> am trying to avoid is "foo\nbar", which I suspect would be
> >> unfriendly to the Cygwin folks.
> >
> > Being unhappy one of them, I think I'd better manage (even if by
> > postprocessing the output).
> >
> > Please, don't make the common case ugly just because of that platform
> > (insanely broken anyway).
> 
> You really have to realize that having LF and TAB in filenames
> are *NOT* the common case, no matter which platform you are
> talking about.
> 

Yes, but "//" in a path is quite common. Even "///" is not uncommon.

How about copy ls' approach were possible?

   -b, --escape, --quoting-style=escape
          Quote  nongraphic  characters in file names using alphabetic and
          octal backslash sequences like those used in C. This  option  is
          the  same as -Q except that filenames are not surrounded by dou-
          ble-quotes.

^ permalink raw reply

* Re: Create object subdirectories on demand
From: Daniel Barkalow @ 2005-10-08  1:45 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <Pine.LNX.4.64.0510061612080.31407@g5.osdl.org>

On Thu, 6 Oct 2005, Linus Torvalds wrote:

> This patch also tries to fix up "write_sha1_from_fd()" to use the new 
> common infrastructure for creating the object files, closing a hole where 
> we might otherwise leave half-written objects in the object database.

This looks right to me, but it would be nice to also split out and 
share the temp file creation. Also, http-fetch.c writes object files and 
needs at least move_temp_to_file() if it's going to do special stuff.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: First cut at git port to Cygwin
From: Elfyn McBratney @ 2005-10-08  1:00 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, git
In-Reply-To: <20051007234547.GC8893@steel.home>

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

On Sat, Oct 08, 2005 at 01:45:47AM +0200, Alex Riesen wrote:
 > Junio C Hamano, Fri, Oct 07, 2005 23:00:02 +0200:
 > > > "Sounds like a thinly veiled threat or a very effective prodding" 8)
 > > > ---
 > > >
 > > > Make read_cache copy the index into memory, to improve portability on
 > > > other OS's which have mmap too, tend to use it less commonly.
 > > >
 > > 
 > > Huh?  where is your memcpy?
 > > 
 > 
 > Junio, unless there already are pressing reasons to put the patch in
 > GIT, could you postpone its inclusion (if you ever considered)? Or at
 > least put "#ifdef __cygwin" (I hope this is the define) around it?

Close ;) - the define is "__CYGWIN__".

Best,
Elfyn

-- 
Elfyn McBratney
Gentoo Developer/Perl Team Lead
beu/irc.freenode.net                            http://dev.gentoo.org/~beu/
+------------O.o--------------------- http://dev.gentoo.org/~beu/pubkey.asc

PGP Key ID: 0x69DF17AD
PGP Key Fingerprint:
  DBD3 B756 ED58 B1B4 47B9  B3BD 8D41 E597 69DF 17AD

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] If NO_MMAP is defined, fake mmap() and munmap()
From: Alex Riesen @ 2005-10-07 23:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vwtkoyjs9.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano, Sat, Oct 08, 2005 01:44:22 +0200:
> > Since some platforms do not support mmap() at all, and others do only just 
> > so, this patch introduces the option to fake mmap() and munmap() by 
> > malloc()ing and read()ing explicitely.
> 
> I guess I can just drop Alex Riesen patch and any other recent
> patches that try to work around mmap().  Happy!
> 

Me too. I was just about to make a read_cache read the whole index in
(absolutely the same as Johannes, but not that elegant).

The platform(s) deserve such a treatment :)

^ permalink raw reply

* Re: [PATCH] close clobbers mmap's errno in read_cache
From: Alex Riesen @ 2005-10-07 23:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <20051007214845.GH8383MdfPADPa@greensroom.kotnet.org>

Sven Verdoolaege, Fri, Oct 07, 2005 23:48:45 +0200:
> On Fri, Oct 07, 2005 at 11:45:51PM +0200, Alex Riesen wrote:
> >  	}
> > +	i = errno;
> >  	close(fd);
> >  	if (map == MAP_FAILED)
> > -		die("index file mmap failed (%s)", strerror(errno));
> > +		die("index file mmap failed (%s)", strerror(i));
> >  
> 
> Why don't you just move the close after the test ?

I don't know.

> There's no point in closing if you're going to die.
> 
> skimo

Of course you're right :) I'm just blind after eight.

^ permalink raw reply

* Re: First cut at git port to Cygwin
From: Alex Riesen @ 2005-10-07 23:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfyrdyre5.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano, Fri, Oct 07, 2005 23:00:02 +0200:
> > "Sounds like a thinly veiled threat or a very effective prodding" 8)
> > ---
> >
> > Make read_cache copy the index into memory, to improve portability on
> > other OS's which have mmap too, tend to use it less commonly.
> >
> 
> Huh?  where is your memcpy?
> 

Junio, unless there already are pressing reasons to put the patch in
GIT, could you postpone its inclusion (if you ever considered)? Or at
least put "#ifdef __cygwin" (I hope this is the define) around it?

It just so ugly... And besides, GIT reportedly works without problems
for many people even without it.

Anyway, the patch is out, so anyone with the problems can just patch
their copy to workaround this specific win2k problem.

Thanks,
Alex

^ permalink raw reply

* Re: [RFC] embedded TAB and LF in pathnames
From: Junio C Hamano @ 2005-10-07 23:44 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Kai Ruemmler
In-Reply-To: <20051007232909.GB8893@steel.home>

Alex Riesen <raa.lkml@gmail.com> writes:

> Junio C Hamano, Fri, Oct 07, 2005 21:35:19 +0200:
>> I have not made up my mind on the exact choice of the quoting
>> convention.  We could say '///' instead of '//', for example, or
>> even '//{LF}//' instead of '//0A' proposed above.  One thing I
>> am trying to avoid is "foo\nbar", which I suspect would be
>> unfriendly to the Cygwin folks.
>
> Being unhappy one of them, I think I'd better manage (even if by
> postprocessing the output).
>
> Please, don't make the common case ugly just because of that platform
> (insanely broken anyway).

You really have to realize that having LF and TAB in filenames
are *NOT* the common case, no matter which platform you are
talking about.

^ permalink raw reply

* Re: [PATCH] If NO_MMAP is defined, fake mmap() and munmap()
From: Junio C Hamano @ 2005-10-07 23:44 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0510080050550.20922@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Since some platforms do not support mmap() at all, and others do only just 
> so, this patch introduces the option to fake mmap() and munmap() by 
> malloc()ing and read()ing explicitely.

I guess I can just drop Alex Riesen patch and any other recent
patches that try to work around mmap().  Happy!

 

^ permalink raw reply

* Re: [RFC] embedded TAB and LF in pathnames
From: Alex Riesen @ 2005-10-07 23:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Kai Ruemmler
In-Reply-To: <7vu0ftyvbc.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano, Fri, Oct 07, 2005 21:35:19 +0200:
> I have not made up my mind on the exact choice of the quoting
> convention.  We could say '///' instead of '//', for example, or
> even '//{LF}//' instead of '//0A' proposed above.  One thing I
> am trying to avoid is "foo\nbar", which I suspect would be
> unfriendly to the Cygwin folks.

Being unhappy one of them, I think I'd better manage (even if by
postprocessing the output).

Please, don't make the common case ugly just because of that platform
(insanely broken anyway).

^ permalink raw reply

* Re: [PATCH] Don't fetch objects that exist in the local repository
From: Johannes Schindelin @ 2005-10-07 23:25 UTC (permalink / raw)
  To: Nick Hengeveld; +Cc: git
In-Reply-To: <20051007230856.GC4989@reactrix.com>

Hi,

On Fri, 7 Oct 2005, Nick Hengeveld wrote:

> On Sat, Oct 08, 2005 at 12:50:53AM +0200, Johannes Schindelin wrote:
> 
> > > Be sure not to fetch objects that already exist in the local repository.
> > 
> > Really? I seem to recall a dispute I had with Linus that this is 
> > unacceptable. He seems worried about incomplete fetches.
> 
> This patch just moved an existing check from the process queue loop to the
> appropriate places in transport-specific code to prevent the transport
> from transferring an object that appeared in the local repository after
> it was prefetched (eg. via a pack), and to make sure that all objects that
> were prefetched are subsequently fetched so the transport can perform the
> appropriate cleanup.

Ah! Okay, now I understand.

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH] Don't fetch objects that exist in the local repository
From: Nick Hengeveld @ 2005-10-07 23:08 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0510080047500.20922@wbgn013.biozentrum.uni-wuerzburg.de>

On Sat, Oct 08, 2005 at 12:50:53AM +0200, Johannes Schindelin wrote:

> > Be sure not to fetch objects that already exist in the local repository.
> 
> Really? I seem to recall a dispute I had with Linus that this is 
> unacceptable. He seems worried about incomplete fetches.

This patch just moved an existing check from the process queue loop to the
appropriate places in transport-specific code to prevent the transport
from transferring an object that appeared in the local repository after
it was prefetched (eg. via a pack), and to make sure that all objects that
were prefetched are subsequently fetched so the transport can perform the
appropriate cleanup.

-- 
For a successful technology, reality must take precedence over public
relations, for nature cannot be fooled.

^ permalink raw reply

* [PATCH] If NO_MMAP is defined, fake mmap() and munmap()
From: Johannes Schindelin @ 2005-10-07 22:55 UTC (permalink / raw)
  To: git; +Cc: junkio


Since some platforms do not support mmap() at all, and others do only just 
so, this patch introduces the option to fake mmap() and munmap() by 
malloc()ing and read()ing explicitely.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

Only lightly tested, but it seems to work correctly (after all, this 
commit was created after compiling with NO_MMAP=1).

 Makefile      |    6 +++
 cache.h       |   16 ++++++++
 compat/mmap.c |  113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 mailsplit.c   |    1 -
 4 files changed, 135 insertions(+), 1 deletions(-)
 create mode 100644 compat/mmap.c

applies-to: 2c165e1b55bf857247c0f074e9bea680bf411586
3d4f1e103b35c28c8190f9a59ffa95221277fdb4
diff --git a/Makefile b/Makefile
index 2f7cdd4..fb4c410 100644
--- a/Makefile
+++ b/Makefile
@@ -27,6 +27,8 @@
 # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
 # Patrick Mauritz).
 #
+# Define NO_MMAP if you want to avoid mmap.
+#
 # Define WITH_OWN_SUBPROCESS_PY if you want to use with python 2.3.
 #
 # Define NO_IPV6 if you lack IPv6 support and getaddrinfo().
@@ -259,6 +261,10 @@ ifdef NO_STRCASESTR
 	DEFINES += -Dstrcasestr=gitstrcasestr
 	LIB_OBJS += compat/strcasestr.o
 endif
+ifdef NO_MMAP
+	DEFINES += -Dmmap=gitfakemmap -Dmunmap=gitfakemunmap -DNO_MMAP
+	LIB_OBJS += compat/mmap.o
+endif
 ifdef NO_IPV6
 	DEFINES += -DNO_IPV6 -Dsockaddr_storage=sockaddr_in
 endif
diff --git a/cache.h b/cache.h
index 514adb8..5987d4c 100644
--- a/cache.h
+++ b/cache.h
@@ -11,7 +11,9 @@
 #include <string.h>
 #include <errno.h>
 #include <limits.h>
+#ifndef NO_MMAP
 #include <sys/mman.h>
+#endif
 #include <sys/param.h>
 #include <netinet/in.h>
 #include <sys/types.h>
@@ -356,4 +358,18 @@ extern void packed_object_info_detail(st
 /* Dumb servers support */
 extern int update_server_info(int);
 
+#ifdef NO_MMAP
+
+#ifndef PROT_READ
+#define PROT_READ 1
+#define PROT_WRITE 2
+#define MAP_PRIVATE 1
+#define MAP_FAILED ((void*)-1)
+#endif
+
+extern void *gitfakemmap(void *start, size_t length, int prot , int flags, int fd, off_t offset);
+extern int gitfakemunmap(void *start, size_t length);
+
+#endif
+
 #endif /* CACHE_H */
diff --git a/compat/mmap.c b/compat/mmap.c
new file mode 100644
index 0000000..fca6321
--- /dev/null
+++ b/compat/mmap.c
@@ -0,0 +1,113 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+#include "../cache.h"
+
+typedef struct fakemmapwritable {
+	void *start;
+	size_t length;
+	int fd;
+	off_t offset;
+	struct fakemmapwritable *next;
+} fakemmapwritable;
+
+static fakemmapwritable *writablelist = NULL;
+
+void *gitfakemmap(void *start, size_t length, int prot , int flags, int fd, off_t offset)
+{
+	int n = 0;
+
+	if(start != NULL)
+		die("Invalid usage of gitfakemmap.");
+
+	if(lseek(fd, offset, SEEK_SET)<0) {
+		errno = EINVAL;
+		return MAP_FAILED;
+	}
+
+	start = xmalloc(length);
+	if(start == NULL) {
+		errno = ENOMEM;
+		return MAP_FAILED;
+	}
+
+	while(n < length) {
+		int count = read(fd, start+n, length-n);
+
+		if(count == 0) {
+			memset(start+n, 0, length-n);
+			break;
+		}
+
+		if(count < 0) {
+			free(start);
+			errno = EACCES;
+			return MAP_FAILED;
+		}
+
+		n += count;
+	}
+
+	if(prot & PROT_WRITE) {
+		fakemmapwritable *next = xmalloc(sizeof(fakemmapwritable));
+		next->start = start;
+		next->length = length;
+		next->fd = dup(fd);
+		next->offset = offset;
+		next->next = writablelist;
+		writablelist = next;
+	}
+
+	return start;
+}
+
+int gitfakemunmap(void *start, size_t length)
+{
+	fakemmapwritable *writable = writablelist, *before = NULL;
+
+	while(writable && (writable->start > start + length
+			|| writable->start + writable->length < start)) {
+		before = writable;
+		writable = writable->next;
+	}
+
+	if(writable) {
+		/* need to write back the contents */
+		int n = 0;
+
+		if(writable->start != start || writable->length != length)
+			die("fakemmap does not support partial write back.");
+
+		if(lseek(writable->fd, writable->offset, SEEK_SET) < 0) {
+			free(start);
+			errno = EBADF;
+			return -1;
+		}
+
+		while(n < length) {
+			int count = write(writable->fd, start + n, length - n);
+
+			if(count < 0) {
+				errno = EINVAL;
+				return -1;
+			}
+
+			n += count;
+		}
+
+		close(writable->fd);
+
+		if(before)
+			before->next = writable->next;
+		else
+			writablelist = writable->next;
+
+		free(writable);
+	}
+
+	free(start);
+
+	return 0;
+}
+
diff --git a/mailsplit.c b/mailsplit.c
index 7981f87..0f8100d 100644
--- a/mailsplit.c
+++ b/mailsplit.c
@@ -9,7 +9,6 @@
 #include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <sys/mman.h>
 #include <string.h>
 #include <stdio.h>
 #include <ctype.h>
---
0.99.8.GIT

^ permalink raw reply related

* Re: [PATCH] Don't fetch objects that exist in the local repository
From: Johannes Schindelin @ 2005-10-07 22:50 UTC (permalink / raw)
  To: Nick Hengeveld; +Cc: git
In-Reply-To: <20051007220151.GB4989@reactrix.com>

Hi,

On Fri, 7 Oct 2005, Nick Hengeveld wrote:

> Be sure not to fetch objects that already exist in the local repository.

Really? I seem to recall a dispute I had with Linus that this is 
unacceptable. He seems worried about incomplete fetches.

Only objects which are descendants of refs are safe, since the refs are 
written only after the objects were fetched successfully.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Add support for parallel HTTP transfers
From: Daniel Barkalow @ 2005-10-07 22:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nick Hengeveld, git
In-Reply-To: <7vachl42tt.fsf@assigned-by-dhcp.cox.net>

On Fri, 7 Oct 2005, Junio C Hamano wrote:

> Nick Hengeveld <nickh@reactrix.com> writes:
> 
> > I think the only downside to leaving that check in place is that when
> > pull() finishes there may be completed requests left behind in the
> > queue, possibly with unreported transfer errors.  Would it make sense
> > to just release any requests left in the queue after pull(), and report
> > if any of them had transfer errors?
> 
> Pull finishing and reporting success while some requests have
> still been outstanding with transfer errors sounds to me that
> decision to finish and declare success is made prematurely.
> What do these leftover requests you are worried about ask for?
> Are you making redundant requests, which can turn out to be
> unneeded?

I believe that the situation is the one you describe in your previous 
message: we determine we need to fetch A and B; we ask for A; we ask for 
B; we find A isn't available alone, but is available in a pack; we get the 
pack; we find we now have B (in the pack); the request for B (which would 
probably fail) is left dangling.

The only actual problem I can see is if this happens with a whole bunch of 
objects at the beginning of a big download, and all but one of your 
connections are left in this state while you download all of the loose 
objects over the one connection that got the pack.

I don't know if this is a problem for the new http code, but it could be 
an issue in general if a transport method allocates resources in 
prefetch().

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* [PATCH] Don't fetch objects that exist in the local repository
From: Nick Hengeveld @ 2005-10-07 22:01 UTC (permalink / raw)
  To: git

Be sure not to fetch objects that already exist in the local repository.
The main process loop no longer performs this check, http-fetch now checks
prior to starting a new request queue entry and when fetch_object() is called,
and local-fetch now checks when fetch_object() is called.

As discussed in this thread: http://marc.theaimsgroup.com/?t=112854890500001

Signed-off-by: Nick Hengeveld <nickh@reactrix.com>


---

 fetch.c       |    2 +-
 http-fetch.c  |   10 +++++++++-
 local-fetch.c |    5 ++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

41b78748fe458224fc1d621f5f0a4df2a3ac3253
diff --git a/fetch.c b/fetch.c
--- a/fetch.c
+++ b/fetch.c
@@ -165,7 +165,7 @@ static int loop(void)
 		 * the queue because we needed to fetch it first.
 		 */
 		if (! (obj->flags & TO_SCAN)) {
-			if (!has_sha1_file(obj->sha1) && fetch(obj->sha1)) {
+			if (fetch(obj->sha1)) {
 				report_missing(obj->type
 					       ? obj->type
 					       : "object", obj->sha1);
diff --git a/http-fetch.c b/http-fetch.c
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -489,7 +489,10 @@ void process_request_queue()
 
 	while (active_requests < max_requests && request != NULL) {
 		if (request->state == WAITING) {
-			start_request(request);
+			if (has_sha1_file(request->sha1))
+				release_request(request);
+			else
+				start_request(request);
 			curl_multi_perform(curlm, &num_transfers);
 		}
 		request = request->next;
@@ -890,6 +893,11 @@ static int fetch_object(struct alt_base 
 	if (request == NULL)
 		return error("Couldn't find request for %s in the queue", hex);
 
+	if (has_sha1_file(request->sha1)) {
+		release_request(request);
+		return 0;
+	}
+
 #ifdef USE_CURL_MULTI
 	int num_transfers;
 	while (request->state == WAITING) {
diff --git a/local-fetch.c b/local-fetch.c
--- a/local-fetch.c
+++ b/local-fetch.c
@@ -166,7 +166,10 @@ static int fetch_file(const unsigned cha
 
 int fetch(unsigned char *sha1)
 {
-	return fetch_file(sha1) && fetch_pack(sha1);
+	if (has_sha1_file(sha1))
+		return 0;
+	else
+		return fetch_file(sha1) && fetch_pack(sha1);
 }
 
 int fetch_ref(char *ref, unsigned char *sha1)

^ permalink raw reply

* Re: [PATCH] close clobbers mmap's errno in read_cache
From: Sven Verdoolaege @ 2005-10-07 21:48 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, git
In-Reply-To: <20051007214551.GA8893@steel.home>

On Fri, Oct 07, 2005 at 11:45:51PM +0200, Alex Riesen wrote:
>  	}
> +	i = errno;
>  	close(fd);
>  	if (map == MAP_FAILED)
> -		die("index file mmap failed (%s)", strerror(errno));
> +		die("index file mmap failed (%s)", strerror(i));
>  

Why don't you just move the close after the test ?
There's no point in closing if you're going to die.

skimo

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox