Re: git_inetd_server: run git-http-backend using inetd
On 07/19/2014 07:06 PM, Jonathan Nieder wrote:
Torsten Bögershausen wrote:
Jonathan, (I'm good in searching, but bad in finding)
could you point out where the source code for the git package for
debian is ?
I recently learned about mDNS, and will probably do some tests
and experiments later, and would like to test the lookup feature
of 0010.
Thanks. It's at git://git.debian.org/~jrnieder-guest/git branch
release+patches and mirrored at http://repo.or.cz/r/git/debian
With my limited reading of the RFC and the code, and without having
test environment, my spontanous (and probably incomplete) change could
look like this:
- Treat host:9418 the same as host
- When the hosts ends with .local, do not use DNS.
static int git_tcp_connect_sock(char *host, int flags)
{
struct strbuf error_message = STRBUF_INIT;
int sockfd = -1, gai = 0;
const char *port = NULL;
struct host *hosts = NULL;
int j, n = 0;
get_host_and_port(host, port);
if (!port)
port = STR(DEFAULT_GIT_PORT);
n = get_srv(host, hosts);
if ((n = 0) ends_with(host, .local)) {
/*
host.local is really local, do not send it to a DNS resolver
The user may try host without .local
*/
die(Unable to look up %s, host);
}
if (!n) {
hosts = xmalloc(sizeof(*hosts));
hosts[0].hostname = xstrdup(host);
hosts[0].port = xstrdup(port);
[snip]
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] add documentation for writing config files
Replace TODO introduced in commit 9c3c22 with documentation explaining Git config API functions for writing configuration files. Signed-off-by: Tanay Abhra tanay...@gmail.com --- Minor nit corrected. Thanks for the review. Documentation/technical/api-config.txt | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..edd5018 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -137,4 +137,33 @@ int read_file_with_include(const char *file, config_fn_t fn, void *data) Writing Config Files -TODO +Git gives multiple entry points in the Config API to write config values to +files namely `git_config_set_in_file` and `git_config_set`, which write to +a specific config file or to `.git/config` respectively. They both take a +key/value pair as parameter. +In the end they both call `git_config_set_multivar_in_file` which takes four +parameters: + +- the name of the file, as a string, to which key/value pairs will be written. + +- the name of key, as a string. This is in canonical flat form: the section, + subsection, and variable segments will be separated by dots, and the section + and variable segments will be all lowercase. + E.g., `core.ignorecase`, `diff.SomeType.textconv`. + +- the value of the variable, as a string. If value is equal to NULL, it will + remove the matching key from the config file. + +- the value regex, as a string. It will disregard key/value pairs where value + does not match. + +- a multi_replace value, as an int. If value is equal to zero, nothing or only + one matching key/value is replaced, else all matching key/values (regardless + how many) are removed, before the new pair is written. + +It returns 0 on success. + +Also, there are functions `git_config_rename_section` and +`git_config_rename_section_in_file` with parameters `old_name` and `new_name` +for renaming or removing sections in the config files. If NULL is passed +through `new_name` parameter, the section will be removed from the config file. -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] abspath.c: use PATH_MAX in real_path_internal()
Am 20.07.2014 02:29, schrieb Karsten Blees: unix-socket.c: This looks pretty broken. The cd / cd back logic is only ever used if the socket path is too long. In this case, after cd'ing to the parent directory of the socket, unix_stream_listen tries to unlink the *original* socket path, instead of the remaining socket basename in sockaddr_un.sun_path. I.e. the subsequent bind() will fail on second invocation of the credential daemon. -- 8 -- Subject: [PATCH] unix-socket: remove stale socket before calling chdir() unix_stream_listen() is given a path. It calls unix_sockaddr_init(), which in turn can call chdir(). After that a relative path doesn't mean the same as before. Any use of the original path should thus happen before that call. For that reason, unlink the given path (to get rid of a possibly existing stale socket) right at the beginning of the function. Noticed-by: Karsten Blees karsten.bl...@gmail.com Signed-off-by: Rene Scharfe l@web.de --- unix-socket.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/unix-socket.c b/unix-socket.c index 01f119f..91bd6b8 100644 --- a/unix-socket.c +++ b/unix-socket.c @@ -99,11 +99,12 @@ int unix_stream_listen(const char *path) struct sockaddr_un sa; struct unix_sockaddr_context ctx; + unlink(path); + if (unix_sockaddr_init(sa, path, ctx) 0) return -1; fd = unix_stream_socket(); - unlink(path); if (bind(fd, (struct sockaddr *)sa, sizeof(sa)) 0) goto fail; -- 2.0.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] abspath.c: use PATH_MAX in real_path_internal()
Am 20.07.2014 01:55, schrieb Karsten Blees: Am 18.07.2014 13:32, schrieb René Scharfe: Am 18.07.2014 01:03, schrieb Karsten Blees: Am 17.07.2014 19:05, schrieb René Scharfe: Am 17.07.2014 14:45, schrieb Nguyễn Thái Ngọc Duy: [...] These routines have traditionally been used by programs to save the name of a working directory for the purpose of returning to it. A much faster and less error-prone method of accomplishing this is to open the current directory (.) and use the fchdir(2) function to return. fchdir() is part of the POSIX-XSI extension, as is realpath(). So why not use realpath() directly (which would also be thread-safe)? That's a good question; thanks for stepping back and looking at the bigger picture. If there is widespread OS support for a functionality then we should use it and just provide a compatibility implementation for those platforms lacking it. The downside is that compat code gets less testing. I just noticed that in contrast to the POSIX realpath(), our real_path() doesn't require the last path component to exist. I don't know if this property is required by the calling code, though. If it's important then removing the last component on ENOENT, calling realpath() again and appending it should handle that, right? Seeing that readlink() You mean realpath()? We don't have a stub for that yet. is left as a stub in compat/mingw.h that only errors out, would the equivalent function on Windows be PathCanonicalize (http://msdn.microsoft.com/en-us/library/windows/desktop/bb773569%28v=vs.85%29.aspx)? PathCanonicalize() doesn't return an absolute path, the realpath() equivalent would be GetFullPathName() (doesn't resolve symlinks) or GetFinalPathNameByHandle() (requires Vista, resolves symlinks, requires the path to exist). I meant readlink() and assumed its ENOSYS stub in compat/mingw.h means that git doesn't handle symlinks on Windows at all. However, a POSIX conformant getcwd must fail with ERANGE if the buffer is too small. So a better alternative would be to add a strbuf_getcwd() that works similar to strbuf_readlink() (i.e. resize the buffer until its large enough). That's a good idea anyway, will send patches. Side note: the 'hard' 260 limit for the current directory also means that as long as we *simulate* realpath() via chdir()/getcwd(), long paths [1] don't work here. Great, so that motivation for getting a chdir()-free implementation remains. :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] getcwd without PATH_MAX
Paths longer than PATH_MAX can be created and used on at least on some file systems. Currently we use getcwd() generally with a PATH_MAX- sized buffer. This short series adds two functions, strbuf_add_cwd() and xgetcwd(), then uses them to reduce the number of fixed-sized buffers and to allow us to handle longer working directory paths. René Scharfe (3): strbuf: add strbuf_add_cwd() wrapper: add xgetcwd() use xgetcwd() get the current directory or die Documentation/technical/api-strbuf.txt | 4 builtin/init-db.c | 17 - builtin/rev-parse.c| 6 +++--- dir.c | 12 git-compat-util.h | 1 + strbuf.c | 22 ++ strbuf.h | 1 + trace.c| 7 --- wrapper.c | 8 9 files changed, 59 insertions(+), 19 deletions(-) -- 2.0.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] strbuf: add strbuf_add_cwd()
Add strbuf_add_cwd(), which adds the current working directory to a
strbuf. Because it doesn't use a fixed-size buffer it supports
arbitrarily long paths, as long as the platform's getcwd() does as
well. At least on Linux and FreeBSD it handles paths longer than
PATH_MAX just fine.
Suggested-by: Karsten Blees karsten.bl...@gmail.com
Signed-off-by: Rene Scharfe l@web.de
---
Documentation/technical/api-strbuf.txt | 4
strbuf.c | 22 ++
strbuf.h | 1 +
3 files changed, 27 insertions(+)
diff --git a/Documentation/technical/api-strbuf.txt
b/Documentation/technical/api-strbuf.txt
index f9c06a7..b96b78c 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -257,6 +257,10 @@ which can be used by the programmer of the callback as she
sees fit.
Add a formatted string to the buffer.
+`strbuf_add_cwd`::
+
+ Add the current working directory to the buffer.
+
`strbuf_commented_addf`::
Add a formatted string prepended by a comment character and a
diff --git a/strbuf.c b/strbuf.c
index 33018d8..4e44773 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -406,6 +406,28 @@ int strbuf_readlink(struct strbuf *sb, const char *path,
size_t hint)
return -1;
}
+int strbuf_add_cwd(struct strbuf *sb)
+{
+ size_t oldalloc = sb-alloc;
+ size_t guessed_len = 32;
+
+ for (;; guessed_len *= 2) {
+ char *cwd;
+
+ strbuf_grow(sb, guessed_len);
+ cwd = getcwd(sb-buf + sb-len, sb-alloc - sb-len);
+ if (cwd) {
+ strbuf_setlen(sb, sb-len + strlen(cwd));
+ return 0;
+ }
+ if (errno != ERANGE)
+ break;
+ }
+ if (oldalloc == 0)
+ strbuf_release(sb);
+ return -1;
+}
+
int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
{
int ch;
diff --git a/strbuf.h b/strbuf.h
index a7c0192..ba95cd6 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -174,6 +174,7 @@ extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);
extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);
extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
+extern int strbuf_add_cwd(struct strbuf *sb);
extern int strbuf_getwholeline(struct strbuf *, FILE *, int);
extern int strbuf_getline(struct strbuf *, FILE *, int);
--
2.0.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] wrapper: add xgetcwd()
Add the helper function xgetcwd(), which returns the current directory
or dies. The returned string has to be free()d after use.
Signed-off-by: Rene Scharfe l@web.de
---
git-compat-util.h | 1 +
wrapper.c | 8
2 files changed, 9 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 0b53c9a..d64d012 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -605,6 +605,7 @@ extern int xmkstemp(char *template);
extern int xmkstemp_mode(char *template, int mode);
extern int odb_mkstemp(char *template, size_t limit, const char *pattern);
extern int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1);
+extern char *xgetcwd(void);
static inline size_t xsize_t(off_t len)
{
diff --git a/wrapper.c b/wrapper.c
index bc1bfb8..e297fa9 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -493,3 +493,11 @@ struct passwd *xgetpwuid_self(void)
errno ? strerror(errno) : _(no such user));
return pw;
}
+
+char *xgetcwd(void)
+{
+ struct strbuf sb = STRBUF_INIT;
+ if (strbuf_add_cwd(sb))
+ die_errno(unable to get current working directory);
+ return strbuf_detach(sb, NULL);
+}
--
2.0.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] use xgetcwd() get the current directory or die
Convert several calls of getcwd() and die() to use xgetcwd() instead.
This way we get rid of fixed-size buffers (which can be too small
depending on the used file system) and gain consistent error messages.
Signed-off-by: Rene Scharfe l@web.de
---
builtin/init-db.c | 17 -
builtin/rev-parse.c | 6 +++---
dir.c | 12
trace.c | 7 ---
4 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 56f85e2..f6dd172 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -426,8 +426,9 @@ int init_db(const char *template_dir, unsigned int flags)
static int guess_repository_type(const char *git_dir)
{
- char cwd[PATH_MAX];
const char *slash;
+ char *cwd;
+ int cwd_is_git_dir;
/*
* GIT_DIR=. git init is always bare.
@@ -435,9 +436,10 @@ static int guess_repository_type(const char *git_dir)
*/
if (!strcmp(., git_dir))
return 1;
- if (!getcwd(cwd, sizeof(cwd)))
- die_errno(_(cannot tell cwd));
- if (!strcmp(git_dir, cwd))
+ cwd = xgetcwd();
+ cwd_is_git_dir = !strcmp(git_dir, cwd);
+ free(cwd);
+ if (cwd_is_git_dir)
return 1;
/*
* GIT_DIR=.git or GIT_DIR=something/.git is usually not.
@@ -572,11 +574,8 @@ int cmd_init_db(int argc, const char **argv, const char
*prefix)
git_work_tree_cfg = xstrdup(real_path(rel));
free(rel);
}
- if (!git_work_tree_cfg) {
- git_work_tree_cfg = xcalloc(PATH_MAX, 1);
- if (!getcwd(git_work_tree_cfg, PATH_MAX))
- die_errno (_(Cannot access current working
directory));
- }
+ if (!git_work_tree_cfg)
+ git_work_tree_cfg = xgetcwd();
if (work_tree)
set_git_work_tree(real_path(work_tree));
else
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 8102aaa..f6bbac7 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -735,7 +735,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
}
if (!strcmp(arg, --git-dir)) {
const char *gitdir =
getenv(GIT_DIR_ENVIRONMENT);
- static char cwd[PATH_MAX];
+ char *cwd;
int len;
if (gitdir) {
puts(gitdir);
@@ -745,10 +745,10 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
puts(.git);
continue;
}
- if (!getcwd(cwd, PATH_MAX))
- die_errno(unable to get current
working directory);
+ cwd = xgetcwd();
len = strlen(cwd);
printf(%s%s.git\n, cwd, len cwd[len-1] !=
'/' ? / : );
+ free(cwd);
continue;
}
if (!strcmp(arg, --resolve-git-dir)) {
diff --git a/dir.c b/dir.c
index e65888d..7b994d4 100644
--- a/dir.c
+++ b/dir.c
@@ -1499,12 +1499,16 @@ int dir_inside_of(const char *subdir, const char *dir)
int is_inside_dir(const char *dir)
{
- char cwd[PATH_MAX];
+ char *cwd;
+ int rc;
+
if (!dir)
return 0;
- if (!getcwd(cwd, sizeof(cwd)))
- die_errno(can't find the current directory);
- return dir_inside_of(cwd, dir) = 0;
+
+ cwd = xgetcwd();
+ rc = (dir_inside_of(cwd, dir) = 0);
+ free(cwd);
+ return rc;
}
int is_empty_dir(const char *path)
diff --git a/trace.c b/trace.c
index 08180a9..3523667 100644
--- a/trace.c
+++ b/trace.c
@@ -158,13 +158,12 @@ void trace_repo_setup(const char *prefix)
{
static const char *key = GIT_TRACE_SETUP;
const char *git_work_tree;
- char cwd[PATH_MAX];
+ char *cwd;
if (!trace_want(key))
return;
- if (!getcwd(cwd, PATH_MAX))
- die(Unable to get current working directory);
+ cwd = xgetcwd();
if (!(git_work_tree = get_git_work_tree()))
git_work_tree = (null);
@@ -176,6 +175,8 @@ void trace_repo_setup(const char *prefix)
trace_printf_key(key, setup: worktree: %s\n,
quote_crnl(git_work_tree));
trace_printf_key(key, setup: cwd: %s\n, quote_crnl(cwd));
trace_printf_key(key, setup: prefix: %s\n, quote_crnl(prefix));
+
+ free(cwd);
}
int trace_want(const char *key)
--
2.0.2
--
To unsubscribe from this
[PATCH v2 1/2] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink)
Something extra is, because struct lock_file is usually used as static
variables in many places. This patch reduces bss section by about 80k
bytes (or 23%) on Linux.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
This helps remove the length check in v1 of the next patch.
cache.h| 2 +-
lockfile.c | 56
2 files changed, 33 insertions(+), 25 deletions(-)
diff --git a/cache.h b/cache.h
index 44aa439..9ecb636 100644
--- a/cache.h
+++ b/cache.h
@@ -554,7 +554,7 @@ struct lock_file {
int fd;
pid_t owner;
char on_list;
- char filename[PATH_MAX];
+ char *filename;
};
#define LOCK_DIE_ON_ERROR 1
#define LOCK_NODEREF 2
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..968b28f 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -7,13 +7,19 @@
static struct lock_file *lock_file_list;
static const char *alternate_index_output;
+static void clear_filename(struct lock_file *lk)
+{
+ free(lk-filename);
+ lk-filename = NULL;
+}
+
static void remove_lock_file(void)
{
pid_t me = getpid();
while (lock_file_list) {
if (lock_file_list-owner == me
- lock_file_list-filename[0]) {
+ lock_file_list-filename) {
if (lock_file_list-fd = 0)
close(lock_file_list-fd);
unlink_or_warn(lock_file_list-filename);
@@ -77,10 +83,16 @@ static char *last_path_elm(char *p)
* Always returns p.
*/
-static char *resolve_symlink(char *p, size_t s)
+static char *resolve_symlink(const char *in)
{
+ static char p[PATH_MAX];
+ size_t s = sizeof(p);
int depth = MAXDEPTH;
+ if (strlen(in) = sizeof(p))
+ return NULL;
+ strcpy(p, in);
+
while (depth--) {
char link[PATH_MAX];
int link_len = readlink(p, link, sizeof(link));
@@ -124,17 +136,12 @@ static char *resolve_symlink(char *p, size_t s)
static int lock_file(struct lock_file *lk, const char *path, int flags)
{
- /*
-* subtract 5 from size to make sure there's room for adding
-* .lock for the lock file name
-*/
- static const size_t max_path_len = sizeof(lk-filename) - 5;
-
- if (strlen(path) = max_path_len)
+ int len;
+ if (!(flags LOCK_NODEREF) !(path = resolve_symlink(path)))
return -1;
+ len = strlen(path) + 5; /* .lock */
+ lk-filename = xmallocz(len);
strcpy(lk-filename, path);
- if (!(flags LOCK_NODEREF))
- resolve_symlink(lk-filename, max_path_len);
strcat(lk-filename, .lock);
lk-fd = open(lk-filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 = lk-fd) {
@@ -153,7 +160,7 @@ static int lock_file(struct lock_file *lk, const char
*path, int flags)
lk-filename);
}
else
- lk-filename[0] = 0;
+ clear_filename(lk);
return lk-fd;
}
@@ -231,16 +238,17 @@ int close_lock_file(struct lock_file *lk)
int commit_lock_file(struct lock_file *lk)
{
- char result_file[PATH_MAX];
- size_t i;
- if (lk-fd = 0 close_lock_file(lk))
+ char *result_file;
+ if ((lk-fd = 0 close_lock_file(lk)) || !lk-filename)
return -1;
- strcpy(result_file, lk-filename);
- i = strlen(result_file) - 5; /* .lock */
- result_file[i] = 0;
- if (rename(lk-filename, result_file))
+ result_file = xmemdupz(lk-filename,
+ strlen(lk-filename) - 5 /* .lock */);
+ if (rename(lk-filename, result_file)) {
+ free(result_file);
return -1;
- lk-filename[0] = 0;
+ }
+ free(result_file);
+ clear_filename(lk);
return 0;
}
@@ -260,11 +268,11 @@ void set_alternate_index_output(const char *name)
int commit_locked_index(struct lock_file *lk)
{
if (alternate_index_output) {
- if (lk-fd = 0 close_lock_file(lk))
+ if ((lk-fd = 0 close_lock_file(lk)) || !lk-filename)
return -1;
if (rename(lk-filename, alternate_index_output))
return -1;
- lk-filename[0] = 0;
+ clear_filename(lk);
return 0;
}
else
@@ -273,10 +281,10 @@ int commit_locked_index(struct lock_file *lk)
void rollback_lock_file(struct lock_file *lk)
{
- if (lk-filename[0]) {
+ if (lk-filename) {
if (lk-fd = 0)
close(lk-fd);
unlink_or_warn(lk-filename);
}
- lk-filename[0] = 0;
+ clear_filename(lk);
}
--
1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at
[PATCH v2 2/2] Make locked paths absolute when current directory is changed
Locked paths are saved in a linked list so that if something wrong
happens, *.lock are removed. This works fine if we keep cwd the same,
which is true 99% of time except:
- update-index and read-tree hold the lock on $GIT_DIR/index really
early, then later on may call setup_work_tree() to move cwd.
- Suppose a lock is being held (e.g. by git add) then somewhere
down the line, somebody calls real_path (e.g. link_alt_odb_entry),
which temporarily moves cwd away and back.
During that time when cwd is moved (either permanently or temporarily)
and we decide to die(), attempts to remove relative *.lock will fail,
and the next operation will complain that some files are still locked.
Avoid this case by turning relative paths to absolute when chdir() is
called (or soon to be called, in setup_git_directory_gently case).
Reported-by: Yue Lin Ho yuelinho...@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Compared to v1, make_locked_paths_absolute() now always succeeds (ok
absolute_path could die inside, but that's a separate problem) and
supports Windows.
abspath.c | 2 +-
cache.h | 6 ++
git.c | 6 +++---
lockfile.c| 12
path.c| 4 ++--
run-command.c | 2 +-
setup.c | 3 ++-
unix-socket.c | 2 +-
8 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/abspath.c b/abspath.c
index ca33558..78c963f 100644
--- a/abspath.c
+++ b/abspath.c
@@ -87,7 +87,7 @@ static const char *real_path_internal(const char *path, int
die_on_error)
goto error_out;
}
- if (chdir(buf)) {
+ if (chdir_safe(buf)) {
if (die_on_error)
die_errno(Could not switch to '%s',
buf);
else
diff --git a/cache.h b/cache.h
index 9ecb636..48ffa21 100644
--- a/cache.h
+++ b/cache.h
@@ -564,6 +564,12 @@ extern int hold_lock_file_for_update(struct lock_file *,
const char *path, int);
extern int hold_lock_file_for_append(struct lock_file *, const char *path,
int);
extern int commit_lock_file(struct lock_file *);
extern void update_index_if_able(struct index_state *, struct lock_file *);
+extern void make_locked_paths_absolute(void);
+static inline int chdir_safe(const char *path)
+{
+ make_locked_paths_absolute();
+ return chdir(path);
+}
extern int hold_locked_index(struct lock_file *, int);
extern int commit_locked_index(struct lock_file *);
diff --git a/git.c b/git.c
index 5b6c761..27766c3 100644
--- a/git.c
+++ b/git.c
@@ -48,7 +48,7 @@ static void save_env(void)
static void restore_env(void)
{
int i;
- if (*orig_cwd chdir(orig_cwd))
+ if (*orig_cwd chdir_safe(orig_cwd))
die_errno(could not move to %s, orig_cwd);
for (i = 0; i ARRAY_SIZE(env_names); i++) {
if (orig_env[i])
@@ -206,7 +206,7 @@ static int handle_options(const char ***argv, int *argc,
int *envchanged)
fprintf(stderr, No directory given for -C.\n
);
usage(git_usage_string);
}
- if (chdir((*argv)[1]))
+ if (chdir_safe((*argv)[1]))
die_errno(Cannot change to '%s', (*argv)[1]);
if (envchanged)
*envchanged = 1;
@@ -292,7 +292,7 @@ static int handle_alias(int *argcp, const char ***argv)
ret = 1;
}
- if (subdir chdir(subdir))
+ if (subdir chdir_safe(subdir))
die_errno(Cannot change to '%s', subdir);
errno = saved_errno;
diff --git a/lockfile.c b/lockfile.c
index 968b28f..cf1e795 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -288,3 +288,15 @@ void rollback_lock_file(struct lock_file *lk)
}
clear_filename(lk);
}
+
+void make_locked_paths_absolute(void)
+{
+ struct lock_file *lk;
+ for (lk = lock_file_list; lk != NULL; lk = lk-next) {
+ if (lk-filename !is_absolute_path(lk-filename)) {
+ char *to_free = lk-filename;
+ lk-filename = xstrdup(absolute_path(lk-filename));
+ free(to_free);
+ }
+ }
+}
diff --git a/path.c b/path.c
index bc804a3..9e8e101 100644
--- a/path.c
+++ b/path.c
@@ -372,11 +372,11 @@ const char *enter_repo(const char *path, int strict)
gitfile = read_gitfile(used_path) ;
if (gitfile)
strcpy(used_path, gitfile);
- if (chdir(used_path))
+ if (chdir_safe(used_path))
return NULL;
path = validated_path;
}
- else if (chdir(path))
+ else if (chdir_safe(path))
return NULL;
if (access(objects, X_OK) == 0 access(refs,
Re: [PATCH 1/3] strbuf: add strbuf_add_cwd()
On Sun, Jul 20, 2014 at 6:21 PM, René Scharfe l@web.de wrote:
+int strbuf_add_cwd(struct strbuf *sb)
+{
+ size_t oldalloc = sb-alloc;
+ size_t guessed_len = 32;
For Linux, I think this is enough to succesfully get cwd in the first
pass. Windows' $HOME is usually deep in C:\Users\Blahblah. Maybe
increase this to 128? And we probably want to keep the guessed value,
so if the first strbuf_add_cwd needs a few rounds to get cwd, the next
strbuf_add_cwd() call does not.
+
+ for (;; guessed_len *= 2) {
+ char *cwd;
+
+ strbuf_grow(sb, guessed_len);
+ cwd = getcwd(sb-buf + sb-len, sb-alloc - sb-len);
+ if (cwd) {
+ strbuf_setlen(sb, sb-len + strlen(cwd));
+ return 0;
+ }
+ if (errno != ERANGE)
+ break;
+ }
+ if (oldalloc == 0)
+ strbuf_release(sb);
+ return -1;
+}
The loop and the following strbuf_release() are correct. But I wonder
if it's easier to read if you save getcwd in a separate/local strbuf
variable and only concat it to sb when you successfully get cwd..
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] wrapper: add xgetcwd()
On Sun, Jul 20, 2014 at 6:21 PM, René Scharfe l@web.de wrote:
+char *xgetcwd(void)
+{
+ struct strbuf sb = STRBUF_INIT;
+ if (strbuf_add_cwd(sb))
+ die_errno(unable to get current working directory);
Wrap the string with _() to make it translatable? I can't see why any
script would want to grep this string..
+ return strbuf_detach(sb, NULL);
+}
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] use xgetcwd() get the current directory or die
On Sun, Jul 20, 2014 at 6:22 PM, René Scharfe l@web.de wrote: Convert several calls of getcwd() and die() to use xgetcwd() instead. This way we get rid of fixed-size buffers (which can be too small depending on the used file system) and gain consistent error messages. Signed-off-by: Rene Scharfe l@web.de --- builtin/init-db.c | 17 - builtin/rev-parse.c | 6 +++--- dir.c | 12 trace.c | 7 --- 4 files changed, 23 insertions(+), 19 deletions(-) There should be a 4/3 to replace the remaining getcwd with strbuf_getcwd. But I guess we could add that to the low hanging fruit list. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink)
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Something extra is, because struct lock_file is usually used as static
variables in many places. This patch reduces bss section by about 80k
bytes (or 23%) on Linux.
This didn't scan for me. Perhaps it's the punctuation. Maybe:
Additionally, because the struct lock_file variables are [were] in many
places static, this patch reduces the bss section size by about 80k
bytes (or 23%) on Linux.
Does my tweak reflect your intent?
Philip.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
This helps remove the length check in v1 of the next patch.
cache.h| 2 +-
lockfile.c | 56
2 files changed, 33 insertions(+), 25 deletions(-)
diff --git a/cache.h b/cache.h
index 44aa439..9ecb636 100644
--- a/cache.h
+++ b/cache.h
@@ -554,7 +554,7 @@ struct lock_file {
int fd;
pid_t owner;
char on_list;
- char filename[PATH_MAX];
+ char *filename;
};
#define LOCK_DIE_ON_ERROR 1
#define LOCK_NODEREF 2
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..968b28f 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -7,13 +7,19 @@
static struct lock_file *lock_file_list;
static const char *alternate_index_output;
+static void clear_filename(struct lock_file *lk)
+{
+ free(lk-filename);
+ lk-filename = NULL;
+}
+
static void remove_lock_file(void)
{
pid_t me = getpid();
while (lock_file_list) {
if (lock_file_list-owner == me
- lock_file_list-filename[0]) {
+ lock_file_list-filename) {
if (lock_file_list-fd = 0)
close(lock_file_list-fd);
unlink_or_warn(lock_file_list-filename);
@@ -77,10 +83,16 @@ static char *last_path_elm(char *p)
* Always returns p.
*/
-static char *resolve_symlink(char *p, size_t s)
+static char *resolve_symlink(const char *in)
{
+ static char p[PATH_MAX];
+ size_t s = sizeof(p);
int depth = MAXDEPTH;
+ if (strlen(in) = sizeof(p))
+ return NULL;
+ strcpy(p, in);
+
while (depth--) {
char link[PATH_MAX];
int link_len = readlink(p, link, sizeof(link));
@@ -124,17 +136,12 @@ static char *resolve_symlink(char *p, size_t s)
static int lock_file(struct lock_file *lk, const char *path, int
flags)
{
- /*
- * subtract 5 from size to make sure there's room for adding
- * .lock for the lock file name
- */
- static const size_t max_path_len = sizeof(lk-filename) - 5;
-
- if (strlen(path) = max_path_len)
+ int len;
+ if (!(flags LOCK_NODEREF) !(path = resolve_symlink(path)))
return -1;
+ len = strlen(path) + 5; /* .lock */
+ lk-filename = xmallocz(len);
strcpy(lk-filename, path);
- if (!(flags LOCK_NODEREF))
- resolve_symlink(lk-filename, max_path_len);
strcat(lk-filename, .lock);
lk-fd = open(lk-filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 = lk-fd) {
@@ -153,7 +160,7 @@ static int lock_file(struct lock_file *lk, const
char *path, int flags)
lk-filename);
}
else
- lk-filename[0] = 0;
+ clear_filename(lk);
return lk-fd;
}
@@ -231,16 +238,17 @@ int close_lock_file(struct lock_file *lk)
int commit_lock_file(struct lock_file *lk)
{
- char result_file[PATH_MAX];
- size_t i;
- if (lk-fd = 0 close_lock_file(lk))
+ char *result_file;
+ if ((lk-fd = 0 close_lock_file(lk)) || !lk-filename)
return -1;
- strcpy(result_file, lk-filename);
- i = strlen(result_file) - 5; /* .lock */
- result_file[i] = 0;
- if (rename(lk-filename, result_file))
+ result_file = xmemdupz(lk-filename,
+strlen(lk-filename) - 5 /* .lock */);
+ if (rename(lk-filename, result_file)) {
+ free(result_file);
return -1;
- lk-filename[0] = 0;
+ }
+ free(result_file);
+ clear_filename(lk);
return 0;
}
@@ -260,11 +268,11 @@ void set_alternate_index_output(const char
*name)
int commit_locked_index(struct lock_file *lk)
{
if (alternate_index_output) {
- if (lk-fd = 0 close_lock_file(lk))
+ if ((lk-fd = 0 close_lock_file(lk)) || !lk-filename)
return -1;
if (rename(lk-filename, alternate_index_output))
return -1;
- lk-filename[0] = 0;
+ clear_filename(lk);
return 0;
}
else
@@ -273,10 +281,10 @@ int commit_locked_index(struct lock_file *lk)
void rollback_lock_file(struct lock_file *lk)
{
- if (lk-filename[0]) {
+ if (lk-filename) {
if (lk-fd = 0)
close(lk-fd);
unlink_or_warn(lk-filename);
}
- lk-filename[0] = 0;
+ clear_filename(lk);
}
--
1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink)
On Sun, Jul 20, 2014 at 7:47 PM, Philip Oakley philipoak...@iee.org wrote: From: Nguyễn Thái Ngọc Duy pclo...@gmail.com Something extra is, because struct lock_file is usually used as static variables in many places. This patch reduces bss section by about 80k bytes (or 23%) on Linux. This didn't scan for me. Perhaps it's the punctuation. Maybe: Additionally, because the struct lock_file variables are [were] in many places static, this patch reduces the bss section size by about 80k bytes (or 23%) on Linux. Does my tweak reflect your intent? Yes. But I should probably put that below the --- line. We're not busybox. 80k is pratically nothing. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] use xgetcwd() get the current directory or die
Am 20.07.2014 14:45, schrieb Duy Nguyen: On Sun, Jul 20, 2014 at 6:22 PM, René Scharfe l@web.de wrote: Convert several calls of getcwd() and die() to use xgetcwd() instead. This way we get rid of fixed-size buffers (which can be too small depending on the used file system) and gain consistent error messages. Signed-off-by: Rene Scharfe l@web.de --- builtin/init-db.c | 17 - builtin/rev-parse.c | 6 +++--- dir.c | 12 trace.c | 7 --- 4 files changed, 23 insertions(+), 19 deletions(-) There should be a 4/3 to replace the remaining getcwd with strbuf_getcwd. But I guess we could add that to the low hanging fruit list. I left out the cases with the go-there-then-come-back pattern on purpose, as they hopefully can be changed to cease using getcwd() in the first place. But I'll include another example (in addition to xgetcwd()) in the reroll. René -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] strbuf: add strbuf_add_cwd()
Am 20.07.2014 14:33, schrieb Duy Nguyen:
On Sun, Jul 20, 2014 at 6:21 PM, René Scharfe l@web.de wrote:
+int strbuf_add_cwd(struct strbuf *sb)
+{
+ size_t oldalloc = sb-alloc;
+ size_t guessed_len = 32;
For Linux, I think this is enough to succesfully get cwd in the first
pass. Windows' $HOME is usually deep in C:\Users\Blahblah. Maybe
increase this to 128? And we probably want to keep the guessed value,
so if the first strbuf_add_cwd needs a few rounds to get cwd, the next
strbuf_add_cwd() call does not.
The initial number is debatable, sure. I just copied the 32 from
strbuf_readline().
C:\Users\John Doe\Documents\Projects\foo (or whatever) isn't thaaat
long either, though. FWIW, the longest (non-hidden) path in my home on
Windows 8.1 is 139 characters long (as reported by dir -r | %{
$_.FullName.Length } | sort -desc | select -f 1), but there are no git
projects in that directory. Not sure if that means 128 would be a
better start value, but I don't mind changing it in any case.
Before adding performance optimizations like remembering the last cwd
length I'd rather see measurements that demonstrate their use. I doubt
we'll see getcwd() show up high on a profile (but didn't measure, except
for a run of t/perf).
+
+ for (;; guessed_len *= 2) {
+ char *cwd;
+
+ strbuf_grow(sb, guessed_len);
+ cwd = getcwd(sb-buf + sb-len, sb-alloc - sb-len);
+ if (cwd) {
+ strbuf_setlen(sb, sb-len + strlen(cwd));
+ return 0;
+ }
+ if (errno != ERANGE)
+ break;
+ }
+ if (oldalloc == 0)
+ strbuf_release(sb);
+ return -1;
+}
The loop and the following strbuf_release() are correct. But I wonder
if it's easier to read if you save getcwd in a separate/local strbuf
variable and only concat it to sb when you successfully get cwd..
Adding an extra allocation and string copy sound more complicated. But
you are right that the function is more complicated than necessary.
Reroll coming..
René
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] wrapper: add xgetcwd()
Am 20.07.2014 14:35, schrieb Duy Nguyen:
On Sun, Jul 20, 2014 at 6:21 PM, René Scharfe l@web.de wrote:
+char *xgetcwd(void)
+{
+ struct strbuf sb = STRBUF_INIT;
+ if (strbuf_add_cwd(sb))
+ die_errno(unable to get current working directory);
Wrap the string with _() to make it translatable? I can't see why any
script would want to grep this string..
Sure, good idea.
+ return strbuf_detach(sb, NULL);
+}
Thank you for the review,
René
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git_inetd_server: run git-http-backend using inetd
On 07/20/2014 08:10 AM, Torsten Bögershausen wrote: On 07/19/2014 07:06 PM, Jonathan Nieder wrote: Torsten Bögershausen wrote: Jonathan, (I'm good in searching, but bad in finding) could you point out where the source code for the git package for debian is ? I recently learned about mDNS, and will probably do some tests and experiments later, and would like to test the lookup feature of 0010. Thanks. It's at git://git.debian.org/~jrnieder-guest/git branch release+patches and mirrored at http://repo.or.cz/r/git/debian I probably need to correct myself: Donwloaded your branch, compiled and tested. On my test system the lookup timed out after 1.9 sec for DNS, and 5 seconds for MDNS (the lookup failed in both cases) I'm not sure any more how to improve things here, and the question remains why Kyle has 15 seconds timeout ? Would it be possible to run wireshark, and give us an example of the URL's you have been using ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/4] getcwd without PATH_MAX
Paths longer than PATH_MAX can be created and used on at least on some file systems. Currently we use getcwd() generally with a PATH_MAX- sized buffer. This series adds two functions, strbuf_getcwd() and xgetcwd(), then uses them to reduce the number of fixed-sized buffers and to allow us to handle longer working directory paths. Not all getcwd() calls are replaced, however. I hope that at least some of the remaining ones can be avoided altogether. If that turns out to be too optimistic then we can use the added function to convert the rest. Changes in v2: * strbuf_getcwd() instead of strbuf_add_cwd(), because the former is simpler and sufficient for now; based on a suggestion by Duy * added patch 2 as an example for strbuf_getcwd() usage, suggested by Duy * made sure strbuf_getcwd() leaves the strbuf intact, no matter what getcwd() does * converted an easy getcwd() call in setup.c René Scharfe (4): strbuf: add strbuf_getcwd() use strbuf_getcwd() to get the current working directory without fixed-sized buffers wrapper: add xgetcwd() use xgetcwd() get the current directory or die Documentation/technical/api-strbuf.txt | 4 builtin/init-db.c | 25 - builtin/rev-parse.c| 6 +++--- dir.c | 12 git-compat-util.h | 1 + git.c | 6 -- setup.c| 6 +++--- strbuf.c | 21 + strbuf.h | 1 + trace.c| 7 --- wrapper.c | 8 11 files changed, 69 insertions(+), 28 deletions(-) -- 2.0.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] strbuf: add strbuf_getcwd()
Add strbuf_getcwd(), which puts the current working directory intto
a strbuf. Because it doesn't use a fixed-size buffer it supports
arbitrarily long paths, as long as the platform's getcwd() does as
well. At least on Linux and FreeBSD it handles paths longer than
PATH_MAX just fine.
Suggested-by: Karsten Blees karsten.bl...@gmail.com
Helped-by: Helped-by: Duy Nguyen pclo...@gmail.com
Signed-off-by: Rene Scharfe l@web.de
---
Documentation/technical/api-strbuf.txt | 4
strbuf.c | 21 +
strbuf.h | 1 +
3 files changed, 26 insertions(+)
diff --git a/Documentation/technical/api-strbuf.txt
b/Documentation/technical/api-strbuf.txt
index f9c06a7..49e477d 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -307,6 +307,10 @@ same behaviour as well.
use it unless you need the correct position in the file
descriptor.
+`strbuf_getcwd`::
+
+ Set the buffer to the path of the current working directory.
+
`stripspace`::
Strip whitespace from a buffer. The second parameter controls if
diff --git a/strbuf.c b/strbuf.c
index 33018d8..2bf4dfa 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -406,6 +406,27 @@ int strbuf_readlink(struct strbuf *sb, const char *path,
size_t hint)
return -1;
}
+int strbuf_getcwd(struct strbuf *sb)
+{
+ size_t oldalloc = sb-alloc;
+ size_t guessed_len = 128;
+
+ for (;; guessed_len *= 2) {
+ strbuf_grow(sb, guessed_len);
+ if (getcwd(sb-buf, sb-alloc)) {
+ strbuf_setlen(sb, strlen(sb-buf));
+ return 0;
+ }
+ if (errno != ERANGE)
+ break;
+ }
+ if (oldalloc == 0)
+ strbuf_release(sb);
+ else
+ strbuf_reset(sb);
+ return -1;
+}
+
int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
{
int ch;
diff --git a/strbuf.h b/strbuf.h
index a7c0192..bc38bb9 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -174,6 +174,7 @@ extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);
extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);
extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
+extern int strbuf_getcwd(struct strbuf *sb);
extern int strbuf_getwholeline(struct strbuf *, FILE *, int);
extern int strbuf_getline(struct strbuf *, FILE *, int);
--
2.0.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] use strbuf_getcwd() to get the current working directory without fixed-sized buffers
Signed-off-by: Rene Scharfe l@web.de
---
builtin/init-db.c | 8
git.c | 6 --
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 56f85e2..c4958b6 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -535,10 +535,10 @@ int cmd_init_db(int argc, const char **argv, const char
*prefix)
usage(init_db_usage[0]);
}
if (is_bare_repository_cfg == 1) {
- static char git_dir[PATH_MAX+1];
-
- setenv(GIT_DIR_ENVIRONMENT,
- getcwd(git_dir, sizeof(git_dir)), argc 0);
+ struct strbuf cwd = STRBUF_INIT;
+ strbuf_getcwd(cwd);
+ setenv(GIT_DIR_ENVIRONMENT, cwd.buf, argc 0);
+ strbuf_release(cwd);
}
if (init_shared_repository != -1)
diff --git a/git.c b/git.c
index 5b6c761..3f52c43 100644
--- a/git.c
+++ b/git.c
@@ -161,9 +161,11 @@ static int handle_options(const char ***argv, int *argc,
int *envchanged)
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, --bare)) {
- static char git_dir[PATH_MAX+1];
+ struct strbuf cwd = STRBUF_INIT;
is_bare_repository_cfg = 1;
- setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
sizeof(git_dir)), 0);
+ strbuf_getcwd(cwd);
+ setenv(GIT_DIR_ENVIRONMENT, cwd.buf, 0);
+ strbuf_release(cwd);
setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 0, 1);
if (envchanged)
*envchanged = 1;
--
2.0.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] wrapper: add xgetcwd()
Add the helper function xgetcwd(), which returns the current directory
or dies. The returned string has to be free()d after use.
Helped-by: Duy Nguyen pclo...@gmail.com
Signed-off-by: Rene Scharfe l@web.de
---
git-compat-util.h | 1 +
wrapper.c | 8
2 files changed, 9 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 0b53c9a..d64d012 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -605,6 +605,7 @@ extern int xmkstemp(char *template);
extern int xmkstemp_mode(char *template, int mode);
extern int odb_mkstemp(char *template, size_t limit, const char *pattern);
extern int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1);
+extern char *xgetcwd(void);
static inline size_t xsize_t(off_t len)
{
diff --git a/wrapper.c b/wrapper.c
index bc1bfb8..bd24cda 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -493,3 +493,11 @@ struct passwd *xgetpwuid_self(void)
errno ? strerror(errno) : _(no such user));
return pw;
}
+
+char *xgetcwd(void)
+{
+ struct strbuf sb = STRBUF_INIT;
+ if (strbuf_getcwd(sb))
+ die_errno(_(unable to get current working directory));
+ return strbuf_detach(sb, NULL);
+}
--
2.0.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] use xgetcwd() get the current directory or die
Convert several calls of getcwd() and die() to use xgetcwd() instead.
This way we get rid of fixed-size buffers (which can be too small
depending on the used file system) and gain consistent error messages.
Signed-off-by: Rene Scharfe l@web.de
---
builtin/init-db.c | 17 -
builtin/rev-parse.c | 6 +++---
dir.c | 12
setup.c | 6 +++---
trace.c | 7 ---
5 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index c4958b6..dc226d1 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -426,8 +426,9 @@ int init_db(const char *template_dir, unsigned int flags)
static int guess_repository_type(const char *git_dir)
{
- char cwd[PATH_MAX];
const char *slash;
+ char *cwd;
+ int cwd_is_git_dir;
/*
* GIT_DIR=. git init is always bare.
@@ -435,9 +436,10 @@ static int guess_repository_type(const char *git_dir)
*/
if (!strcmp(., git_dir))
return 1;
- if (!getcwd(cwd, sizeof(cwd)))
- die_errno(_(cannot tell cwd));
- if (!strcmp(git_dir, cwd))
+ cwd = xgetcwd();
+ cwd_is_git_dir = !strcmp(git_dir, cwd);
+ free(cwd);
+ if (cwd_is_git_dir)
return 1;
/*
* GIT_DIR=.git or GIT_DIR=something/.git is usually not.
@@ -572,11 +574,8 @@ int cmd_init_db(int argc, const char **argv, const char
*prefix)
git_work_tree_cfg = xstrdup(real_path(rel));
free(rel);
}
- if (!git_work_tree_cfg) {
- git_work_tree_cfg = xcalloc(PATH_MAX, 1);
- if (!getcwd(git_work_tree_cfg, PATH_MAX))
- die_errno (_(Cannot access current working
directory));
- }
+ if (!git_work_tree_cfg)
+ git_work_tree_cfg = xgetcwd();
if (work_tree)
set_git_work_tree(real_path(work_tree));
else
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 8102aaa..f6bbac7 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -735,7 +735,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
}
if (!strcmp(arg, --git-dir)) {
const char *gitdir =
getenv(GIT_DIR_ENVIRONMENT);
- static char cwd[PATH_MAX];
+ char *cwd;
int len;
if (gitdir) {
puts(gitdir);
@@ -745,10 +745,10 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
puts(.git);
continue;
}
- if (!getcwd(cwd, PATH_MAX))
- die_errno(unable to get current
working directory);
+ cwd = xgetcwd();
len = strlen(cwd);
printf(%s%s.git\n, cwd, len cwd[len-1] !=
'/' ? / : );
+ free(cwd);
continue;
}
if (!strcmp(arg, --resolve-git-dir)) {
diff --git a/dir.c b/dir.c
index e65888d..7b994d4 100644
--- a/dir.c
+++ b/dir.c
@@ -1499,12 +1499,16 @@ int dir_inside_of(const char *subdir, const char *dir)
int is_inside_dir(const char *dir)
{
- char cwd[PATH_MAX];
+ char *cwd;
+ int rc;
+
if (!dir)
return 0;
- if (!getcwd(cwd, sizeof(cwd)))
- die_errno(can't find the current directory);
- return dir_inside_of(cwd, dir) = 0;
+
+ cwd = xgetcwd();
+ rc = (dir_inside_of(cwd, dir) = 0);
+ free(cwd);
+ return rc;
}
int is_empty_dir(const char *path)
diff --git a/setup.c b/setup.c
index 0a22f8b..dc92d63 100644
--- a/setup.c
+++ b/setup.c
@@ -434,16 +434,16 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
if (is_absolute_path(git_work_tree_cfg))
set_git_work_tree(git_work_tree_cfg);
else {
- char core_worktree[PATH_MAX];
+ char *core_worktree;
if (chdir(gitdirenv))
die_errno(Could not chdir to '%s', gitdirenv);
if (chdir(git_work_tree_cfg))
die_errno(Could not chdir to '%s',
git_work_tree_cfg);
- if (!getcwd(core_worktree, PATH_MAX))
- die_errno(Could not get directory '%s',
git_work_tree_cfg);
+ core_worktree = xgetcwd();
Re: [PATCH] Add failing test: fsck survives inflate errors
The following message is a courtesy copy of an article
that has been posted to gmane.comp.version-control.git as well.
Oh, I forgot to provide any analysis of the problem. Oops.
It may be just as well, though; I was tired enough that I might have
botched it in any case. So, have an analysis:
While inflate errors are obviously NOT GOOD, and should perhaps be
instantly fatal for most commands, git fsck is something of a special
case because it is useful to have *it* report as many corrupt objects as
possible in one run.
Unfortunately, this is not currently the case, as shown by the provided
testcase.
The output for this testcase is:
,
| checking known breakage:
| hash1=
| hash2=fffe
| mkdir -p .git/objects/ff
| echo not-zlib $(sha1_file $hash1)
| test_when_finished remove_object $hash1
| echo not-zlib $(sha1_file $hash2)
| test_when_finished remove_object $hash2
|
| # Return value is not documented
| test_might_fail git fsck 2out
| cat out echo ==
| grep $hash1.*corrupt out
| grep $hash2.*corrupt out
|
| error: inflate: data stream error (incorrect header check)
| error: unable to unpack header
| error: inflate: data stream error (incorrect header check)
| fatal: loose object (stored in
.git/objects/ff/ff) is corrupt
| ==
| fatal: loose object (stored in
.git/objects/ff/ff) is corrupt
| not ok 5 - fsck survives inflate errors # TODO known breakage
`
If I flip it from expect_failure to expect_success and run the test with
-i, then go into the trash directory and run gdb ../../git-fsck, I can
obtain this (thoroughly rehearsed trimmed) gdb transcript:
,
| % gdb ../../git-fsck
| GNU gdb (Debian 7.7.1-3) 7.7.1
...
| Reading symbols from ../../git-fsck...done.
| (gdb) break error
| Breakpoint 1 at 0x813d24c: file usage.c, line 143.
| (gdb) break die
| Breakpoint 2 at 0x813d152: file usage.c, line 94.
| (gdb) run
| Starting program: /home/naesten/hacking/git/git-fsck
| [Thread debugging using libthread_db enabled]
| Using host libthread_db library
/lib/i386-linux-gnu/i686/cmov/libthread_db.so.1.
| Checking object directories: 100% (256/256), done.
|
| Breakpoint 1, error (err=0x8182f7a inflate: %s (%s)) at usage.c:143
| 143 {
| (gdb) bt
| #0 error (err=0x8182f7a inflate: %s (%s)) at usage.c:143
| #1 0x081452ff in git_inflate (strm=0xbfffe6b8, flush=0)
| at zlib.c:144
| #2 0x08125367 in unpack_sha1_header (stream=optimized out,
| map=optimized out, mapsize=optimized out,
| buffer=optimized out, bufsiz=optimized out)
| at sha1_file.c:1515
| #3 0x08125546 in sha1_loose_object_info (
| sha1=sha1@entry=0x82659d4 '\377' repeats 20 times,
| oi=oi@entry=0xbfffe788) at sha1_file.c:2528
| #4 0x08126b2d in sha1_object_info_extended (
| sha1=0x82659d4 '\377' repeats 20 times, oi=0xbfffe788, flags=1)
| at sha1_file.c:2565
| #5 0x0812666f in sha1_object_info (
| sha1=0x82659d4 '\377' repeats 20 times, sizep=0x0)
| at sha1_file.c:2601
| #6 0x080f6941 in parse_object (
| sha1=0x82659d4 '\377' repeats 20 times) at object.c:247
| #7 0x080758ac in fsck_sha1 (
| sha1=sha1@entry=0x82659d4 '\377' repeats 20 times)
| at builtin/fsck.c:333
...
| (gdb) c
| Continuing.
| error: inflate: data stream error (incorrect header check)
|
| Breakpoint 1, error (err=0x817c525 unable to unpack %s header)
| at usage.c:143
| 143 {
| (gdb) bt
| #0 error (err=0x817c525 unable to unpack %s header) at usage.c:143
| #1 0x08125564 in sha1_loose_object_info (
| sha1=sha1@entry=0x82659d4 '\377' repeats 20 times,
| oi=oi@entry=0xbfffe788) at sha1_file.c:2529
| #2 0x08126b2d in sha1_object_info_extended (
| sha1=0x82659d4 '\377' repeats 20 times, oi=0xbfffe788, flags=1)
| at sha1_file.c:2565
| #3 0x0812666f in sha1_object_info (
| sha1=0x82659d4 '\377' repeats 20 times, sizep=0x0)
| at sha1_file.c:2601
| #4 0x080f6941 in parse_object (
| sha1=0x82659d4 '\377' repeats 20 times) at object.c:247
...
| (gdb) frame 4
| #4 0x080f6941 in parse_object (
| sha1=0x82659d4 '\377' repeats 20 times) at object.c:247
| warning: Source file is more recent than executable.
| 247 sha1_object_info(sha1, NULL) == OBJ_BLOB)) { // --
first error
| (gdb) down
| #3 0x0812666f in sha1_object_info (
| sha1=0x82659d4 '\377' repeats 20 times, sizep=0x0)
| at sha1_file.c:2601
| 2601if (sha1_object_info_extended(sha1, oi,
LOOKUP_REPLACE_OBJECT) 0)
| (gdb) fin
| Run till exit from #3 0x0812666f in sha1_object_info (
| sha1=0x82659d4 '\377' repeats 20 times, sizep=0x0)
| at sha1_file.c:2601
| error: unable to unpack
Re: [PATCH v1] rebase --root: sentinel commit cloaks empty commits
Thomas Rast t...@thomasrast.ch wrote: Please take a closer look at the last two test cases that specify the expected behaviour of rebasing a branch that tracks the empty tree. At this point they expect the Nothing to do error (aborts with untouched history). This is consistent with rebasing only empty commits without `--root`, which also doesn't just delete them from the history. Furthermore, I think the two alternatives adding a note that all commits in the range were empty, and removing the empty commits (thus making the branch empty) are better discussed in a separate bug report. Makes sense to me, though I have never thought much about rebasing empty commits. Maybe Chris has a more informed opinion? I definitely agree with you both that --root should be (and isn't) consistent with normal interactive rebasing. The difference isn't deliberate on my part. On a personal note, I've always disliked the way interactive rebase stops when you pick an existing empty commit or empty log message rather than preserving it. Jumping through a few hoops is perhaps sensible when you create that kind of strange commit, but just annoying when picking an existing empty/logless commit as part of a series. But as you say, that's a separate issue than --root behaving differently to non --root. Cheers, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
scan.coverity: improve the modeling file of git.git
Hi Sam, John and Jeff,
I'm writing to you, as you're listed as the
administrator of the git.git project
on scan.coverity.com
We're currently seeing lots of false positives
as the xmalloc/xrealloc function is handled not properly
by coverity. There are lots of errors Allocation too small for type
Quoting (starting linenumbers are code) from some xrealloc ocurrence:
95void *xrealloc(void *ptr, size_t size)
96{
97void *ret;
98
99memory_limit_check(size);
100ret = realloc(ptr, size);
1. Condition !ret, taking true branch
2. Condition !size, taking true branch
101if (!ret !size)
3. buffer_alloc: realloc(void *, size_t) which allocates 1 bytes based on
1UL.
4. var_assign: Assigning: ret = storage allocated by realloc(ptr, 1UL).
102ret = realloc(ptr, 1);
5. Condition !ret, taking false branch
103if (!ret) {
104try_to_free_routine(size);
105ret = realloc(ptr, size);
106if (!ret !size)
107ret = realloc(ptr, 1);
108if (!ret)
109die(Out of memory, realloc failed);
110}
6. return_dbuffer: Returning allocated array ret.
111return ret;
112}
However I have reviewed the function and I'd be pretty sure it would work as
expected.
According to https://scan.coverity.com/tune we can upload a modelling file,
which will allow us to supress such false positive errors.
I believe we'd need to put in the modelling file something like:
// coverity[+alloc]
void *xrealloc(void *ptr, size_t size);
and that should do. We'd not need to modify the git.git sources,
but just add such a declaration to the modelling file.
Does anyone of you administrators want to experiment with that?
Cheers,
Stefan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 23/31] checkout: clean up half-prepared directories in --to mode
On Sun, Jul 13, 2014 at 12:51 AM, Nguyễn Thái Ngọc Duy
pclo...@gmail.com wrote:
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/checkout.c | 49 +++--
1 file changed, 47 insertions(+), 2 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 81adb74..3167fd2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -20,6 +20,7 @@
#include resolve-undo.h
#include submodule.h
#include argv-array.h
+#include sigchain.h
static const char * const checkout_usage[] = {
N_(git checkout [options] branch),
@@ -814,6 +815,35 @@ static int switch_branches(const struct checkout_opts
*opts,
return ret || writeout_error;
}
+static const char *junk_work_tree;
+static const char *junk_git_dir;
+static int is_junk;
+static pid_t junk_pid;
+
+static void remove_junk(void)
+{
+ struct strbuf sb = STRBUF_INIT;
+ if (!is_junk || getpid() != junk_pid)
+ return;
+ if (junk_git_dir) {
+ strbuf_addstr(sb, junk_git_dir);
+ remove_dir_recursively(sb, 0);
+ strbuf_reset(sb);
+ }
+ if (junk_work_tree) {
+ strbuf_addstr(sb, junk_work_tree);
+ remove_dir_recursively(sb, 0);
+ }
+ strbuf_release(sb);
+}
+
+static void remove_junk_on_signal(int signo)
+{
+ remove_junk();
+ sigchain_pop(signo);
+ raise(signo);
+}
+
static int prepare_linked_checkout(const struct checkout_opts *opts,
struct branch_info *new)
{
@@ -822,7 +852,7 @@ static int prepare_linked_checkout(const struct
checkout_opts *opts,
const char *path = opts-new_worktree, *name;
struct stat st;
struct child_process cp;
- int counter = 0, len;
+ int counter = 0, len, ret;
if (!new-commit)
die(_(no branch specified));
@@ -848,13 +878,21 @@ static int prepare_linked_checkout(const struct
checkout_opts *opts,
strbuf_addf(sb_repo, %d, counter);
}
name = strrchr(sb_repo.buf, '/') + 1;
+
+ junk_pid = getpid();
+ atexit(remove_junk);
+ sigchain_push_common(remove_junk_on_signal);
+
if (mkdir(sb_repo.buf, 0777))
die_errno(_(could not create directory of '%s'),
sb_repo.buf);
+ junk_git_dir = sb_repo.buf;
I've managed to convince myself that, although junk_git_dir becomes a
dangling pointer by the end of prepare_linked_checkout(), it should
never afterward be accessed. Perhaps it would make sense to make this
easier to follow by clearing junk_git_dir when is_junk is cleared?
+ is_junk = 1;
strbuf_addf(sb_git, %s/.git, path);
if (safe_create_leading_directories_const(sb_git.buf))
die_errno(_(could not create leading directories of '%s'),
sb_git.buf);
+ junk_work_tree = path;
write_file(sb_git.buf, 1, gitdir: %s/repos/%s\n,
real_path(get_git_common_dir()), name);
@@ -879,7 +917,14 @@ static int prepare_linked_checkout(const struct
checkout_opts *opts,
memset(cp, 0, sizeof(cp));
cp.git_cmd = 1;
cp.argv = opts-saved_argv;
- return run_command(cp);
+ ret = run_command(cp);
+ if (!ret)
+ is_junk = 0;
Here: perhaps also set is_junk_dir to NULL since it otherwise is about
to become a dangling pointer.
+ strbuf_release(sb);
+ strbuf_release(sb_repo);
+ strbuf_release(sb_git);
+ return ret;
+
}
static int git_checkout_config(const char *var, const char *value, void *cb)
--
1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] abspath.c: use PATH_MAX in real_path_internal()
On Sun, Jul 20, 2014 at 10:00:41AM +0200, René Scharfe wrote: -- 8 -- Subject: [PATCH] unix-socket: remove stale socket before calling chdir() unix_stream_listen() is given a path. It calls unix_sockaddr_init(), which in turn can call chdir(). After that a relative path doesn't mean the same as before. Any use of the original path should thus happen before that call. For that reason, unlink the given path (to get rid of a possibly existing stale socket) right at the beginning of the function. Thanks, I think this ordering problem was just missed in 1eb10f4 (unix-socket: handle long socket pathnames, 2012-01-09). Your solution looks OK, though I think also just using: unlink(sa.sun_path); would work, too (that is the path we are feeding to bind(), whether we have chdir'd or not, so perhaps it is a little more obviously correct?). I'm OK with either. diff --git a/unix-socket.c b/unix-socket.c index 01f119f..91bd6b8 100644 --- a/unix-socket.c +++ b/unix-socket.c @@ -99,11 +99,12 @@ int unix_stream_listen(const char *path) struct sockaddr_un sa; struct unix_sockaddr_context ctx; + unlink(path); + if (unix_sockaddr_init(sa, path, ctx) 0) return -1; fd = unix_stream_socket(); - unlink(path); I briefly wondered if this should be unlinking only when we get EEXIST, but I don't think it is worth caring about. The only caller is credential-cache, and it always wants to unconditionally replace whatever is there (it will already have tried to contact any existing socket). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] use strbuf_getcwd() to get the current working directory without fixed-sized buffers
On Sun, Jul 20, 2014 at 06:49:54PM +0200, René Scharfe wrote:
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 56f85e2..c4958b6 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -535,10 +535,10 @@ int cmd_init_db(int argc, const char **argv, const char
*prefix)
usage(init_db_usage[0]);
}
if (is_bare_repository_cfg == 1) {
- static char git_dir[PATH_MAX+1];
-
- setenv(GIT_DIR_ENVIRONMENT,
- getcwd(git_dir, sizeof(git_dir)), argc 0);
+ struct strbuf cwd = STRBUF_INIT;
+ strbuf_getcwd(cwd);
+ setenv(GIT_DIR_ENVIRONMENT, cwd.buf, argc 0);
+ strbuf_release(cwd);
Hmm. You are not making anything worse here, as we already do not check
the return value of getcwd. But what happens if it fails? Looks like we
currently get a segfault, and the new code will silently set the
variable to the empty string. Neither is particularly helpful.
Should we be using the xgetcwd helper that you add in the next patch?
- setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
sizeof(git_dir)), 0);
+ strbuf_getcwd(cwd);
+ setenv(GIT_DIR_ENVIRONMENT, cwd.buf, 0);
+ strbuf_release(cwd);
Ditto here.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 23/31] checkout: clean up half-prepared directories in --to mode
On Sun, Jul 20, 2014 at 7:55 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Sun, Jul 13, 2014 at 12:51 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: + + junk_pid = getpid(); + atexit(remove_junk); + sigchain_push_common(remove_junk_on_signal); + if (mkdir(sb_repo.buf, 0777)) die_errno(_(could not create directory of '%s'), sb_repo.buf); + junk_git_dir = sb_repo.buf; I've managed to convince myself that, although junk_git_dir becomes a dangling pointer by the end of prepare_linked_checkout(), it should never afterward be accessed. Perhaps it would make sense to make this easier to follow by clearing junk_git_dir when is_junk is cleared? + is_junk = 1; strbuf_addf(sb_git, %s/.git, path); if (safe_create_leading_directories_const(sb_git.buf)) die_errno(_(could not create leading directories of '%s'), sb_git.buf); + junk_work_tree = path; write_file(sb_git.buf, 1, gitdir: %s/repos/%s\n, real_path(get_git_common_dir()), name); @@ -879,7 +917,14 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, memset(cp, 0, sizeof(cp)); cp.git_cmd = 1; cp.argv = opts-saved_argv; - return run_command(cp); + ret = run_command(cp); + if (!ret) + is_junk = 0; Here: perhaps also set is_junk_dir to NULL since it otherwise is about to become a dangling pointer. My bad: s/is_junk_dir/junk_git_dir/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: inotify support, nearly there
Duy Nguyen pclouds at gmail.com writes: Just a quick update for the enthusiasts. My branch file-watcher [1] has got working per-user inotify support. It's a 20 patch series so I'll refrain from spamming git at vger for a while, even though it hurts your eyes a lot less than what I have posted so far. The test suite ran fine with it so it's not that buggy. It has new tests too, even though real inotify is not tested in the new tests. Documentation is there, either in .txt or comments. Using it is simple: $ mkdir ~/.watcher $ git file-watcher --detach ~/.watcher $ git config --global filewatcher.path $HOME/.watcher Will this mean that Git would work faster with repositories with large number of files or commits? I am new into this topic, but I am trying to understand, any pointers are appreciated. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
