From c4712e4553f13d87d655325a57538f299402d457 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sun, 24 Dec 2006 00:47:23 -0500 Subject: Replace mmap with xmmap, better handling MAP_FAILED. In some cases we did not even bother to check the return value of mmap() and just assume it worked. This is bad, because if we are out of virtual address space the kernel returned MAP_FAILED and we would attempt to dereference that address, segfaulting without any real error output to the user. We are replacing all calls to mmap() with xmmap() and moving all MAP_FAILED checking into that single location. If a mmap call fails we try to release enough least-recently-used pack windows to possibly succeed, then retry the mmap() attempt. If we cannot mmap even after releasing pack memory then we die() as none of our callers have any reasonable recovery strategy for a failed mmap. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index e88ed8b2d3..121774cb32 100644 --- a/refs.c +++ b/refs.c @@ -1026,7 +1026,7 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char * fstat(logfd, &st); if (!st.st_size) die("Log %s is empty.", logfile); - logdata = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, logfd, 0); + logdata = xmmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, logfd, 0); close(logfd); lastrec = NULL; -- cgit v1.2.3 From 510c5a8ee392fd2ab954b676bae486b53f444013 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 7 Jan 2007 01:35:34 -0800 Subject: Move initialization of log_all_ref_updates The patches to prevent Porcelainish that require working tree from doing any damage in a bare repository make a lot of sense, and I want to make the is_bare_git_dir() function more reliable. In order to allow the repository owner override the heuristic implemented in is_bare_git_dir() if/when it misidentifies a particular repository, it would make sense to introduce a new configuration variable "[core] bare = true/false", and make is_bare_git_dir() notice it. The scripts would do a 'repo-config --bool --get core.bare' and iff the command fails (i.e. there is no such variable in the configuration file), it would use the heuristic implemented at the script level [*1*]. However, setup_git_env() which is called a lot earlier than we even read from the repository configuration currently makes a call to is_bare_git_dir(), in order to change the default setting for log_all_ref_updates. It somehow feels that this is a hack. By the way, [*1*] is another thing I hate about the current config mechanism. "git-repo-config --get" does not know what the possible configuration variables are, let alone what the default values for them are. It allows us not to maintain a centralized configuration table, which makes it easy to introduce ad-hoc variables and gives a warm fuzzy feeling of being modular, but my feeling is that it is turning out to be a rather high price to pay for scripts. Signed-off-by: Junio C Hamano --- refs.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 52057455a2..b5eee11bd5 100644 --- a/refs.c +++ b/refs.c @@ -923,6 +923,9 @@ static int log_ref_write(struct ref_lock *lock, char *logrec; const char *committer; + if (log_all_ref_updates < 0) + log_all_ref_updates = !is_bare_git_dir(get_git_dir()); + if (log_all_ref_updates && (!strncmp(lock->ref_name, "refs/heads/", 11) || !strncmp(lock->ref_name, "refs/remotes/", 13))) { -- cgit v1.2.3 From 7d1864ce67d83485cf5cbc8c90fc170ee884ef16 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 7 Jan 2007 02:00:28 -0800 Subject: Introduce is_bare_repository() and core.bare configuration variable This removes the old is_bare_git_dir(const char *) to ask if a directory, if it is a GIT_DIR, is a bare repository, and replaces it with is_bare_repository(void *). The function looks at core.bare configuration variable if exists but uses the old heuristics: if it is ".git" or ends with "/.git", then it does not look like a bare repository, otherwise it does. Signed-off-by: Junio C Hamano --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index b5eee11bd5..499086ba61 100644 --- a/refs.c +++ b/refs.c @@ -924,7 +924,7 @@ static int log_ref_write(struct ref_lock *lock, const char *committer; if (log_all_ref_updates < 0) - log_all_ref_updates = !is_bare_git_dir(get_git_dir()); + log_all_ref_updates = !is_bare_repository(); if (log_all_ref_updates && (!strncmp(lock->ref_name, "refs/heads/", 11) || -- cgit v1.2.3 From 93d26e4cb9cec2eb8abb4f37e6dda2c86fcceeac Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Mon, 8 Jan 2007 15:58:08 +0000 Subject: short i/o: fix calls to read to use xread or read_in_full We have a number of badly checked read() calls. Often we are expecting read() to read exactly the size we requested or fail, this fails to handle interrupts or short reads. Add a read_in_full() providing those semantics. Otherwise we at a minimum need to check for EINTR and EAGAIN, where this is appropriate use xread(). Signed-off-by: Andy Whitcroft Signed-off-by: Junio C Hamano --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 52057455a2..2b69e1eddc 100644 --- a/refs.c +++ b/refs.c @@ -284,7 +284,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int * fd = open(path, O_RDONLY); if (fd < 0) return NULL; - len = read(fd, buffer, sizeof(buffer)-1); + len = read_in_full(fd, buffer, sizeof(buffer)-1); close(fd); /* -- cgit v1.2.3 From 93822c2239a336e5cb583549071c59202ef6c5b2 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Mon, 8 Jan 2007 15:58:23 +0000 Subject: short i/o: fix calls to write to use xwrite or write_in_full We have a number of badly checked write() calls. Often we are expecting write() to write exactly the size we requested or fail, this fails to handle interrupts or short writes. Switch to using the new write_in_full(). Otherwise we at a minimum need to check for EINTR and EAGAIN, where this is appropriate use xwrite(). Note, the changes to config handling are much larger and handled in the next patch in the sequence. Signed-off-by: Andy Whitcroft Signed-off-by: Junio C Hamano --- refs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 2b69e1eddc..4d6fad8879 100644 --- a/refs.c +++ b/refs.c @@ -332,7 +332,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master) } lockpath = mkpath("%s.lock", git_HEAD); fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666); - written = write(fd, ref, len); + written = write_in_full(fd, ref, len); close(fd); if (written != len) { unlink(lockpath); @@ -968,7 +968,7 @@ static int log_ref_write(struct ref_lock *lock, sha1_to_hex(sha1), committer); } - written = len <= maxlen ? write(logfd, logrec, len) : -1; + written = len <= maxlen ? write_in_full(logfd, logrec, len) : -1; free(logrec); close(logfd); if (written != len) @@ -987,8 +987,8 @@ int write_ref_sha1(struct ref_lock *lock, unlock_ref(lock); return 0; } - if (write(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 || - write(lock->lock_fd, &term, 1) != 1 + if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 || + write_in_full(lock->lock_fd, &term, 1) != 1 || close(lock->lock_fd) < 0) { error("Couldn't write %s", lock->lk->filename); unlock_ref(lock); -- cgit v1.2.3 From 883d60fa97c6397450fb129634054e0a6101baac Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 8 Jan 2007 01:59:54 +0100 Subject: Sanitize for_each_reflog_ent() It used to ignore the return value of the helper function; now, it expects it to return 0, and stops iteration upon non-zero return values; this value is then passed on as the return value of for_each_reflog_ent(). Further, it makes no sense to force the parsing upon the helper functions; for_each_reflog_ent() now calls the helper function with old and new sha1, the email, the timestamp & timezone, and the message. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- refs.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 4d6fad8879..d189d8ad99 100644 --- a/refs.c +++ b/refs.c @@ -1097,7 +1097,7 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char * return 0; } -void for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data) +int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data) { const char *logfile; FILE *logfp; @@ -1106,19 +1106,35 @@ void for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data) logfile = git_path("logs/%s", ref); logfp = fopen(logfile, "r"); if (!logfp) - return; + return -1; while (fgets(buf, sizeof(buf), logfp)) { unsigned char osha1[20], nsha1[20]; - int len; + char *email_end, *message; + unsigned long timestamp; + int len, ret, tz; /* old SP new SP name SP time TAB msg LF */ len = strlen(buf); if (len < 83 || buf[len-1] != '\n' || get_sha1_hex(buf, osha1) || buf[40] != ' ' || - get_sha1_hex(buf + 41, nsha1) || buf[81] != ' ') + get_sha1_hex(buf + 41, nsha1) || buf[81] != ' ' || + !(email_end = strchr(buf + 82, '>')) || + email_end[1] != ' ' || + !(timestamp = strtoul(email_end + 2, &message, 10)) || + !message || message[0] != ' ' || + (message[1] != '+' && message[1] != '-') || + !isdigit(message[2]) || !isdigit(message[3]) || + !isdigit(message[4]) || !isdigit(message[5]) || + message[6] != '\t') continue; /* corrupt? */ - fn(osha1, nsha1, buf+82, cb_data); + email_end[1] = '\0'; + tz = strtol(message + 1, NULL, 10); + message += 7; + ret = fn(osha1, nsha1, buf+82, timestamp, tz, message, cb_data); + if (ret) + return ret; } fclose(logfp); + return 0; } -- cgit v1.2.3