Re: [PATCH v6b 5/8] branch: drop non-commit error reporting

2015-09-24 Thread Matthieu Moy
Karthik Nayak writes: > Remove the error "branch '%s' does not point at a commit" in > apppend_ref() which reports branch refs which do not point to > commits. Also remove the error "some refs could not be read" in > print_ref_list() which is triggered as a consequence of the first > error. > > T

Re: [RFC/PATCH v1] Add Travis CI support

2015-09-24 Thread Dennis Kaarsemaker
On do, 2015-09-24 at 17:41 -0700, Junio C Hamano wrote: > larsxschnei...@gmail.com writes: > > > My idea is that the owner of "https://github.com/git/git"; enables this > > account > > for Travis (it's free!). Then we would automatically get the test state for > > all > > official branches. > >

Teak Furniture

2015-09-24 Thread Junira
Teak Furniture, Famous teak wood is very strong and durable, therefore many processed into garden furniture, interior furniture and handicrafts. To see a variety of teak wood furniture you can visit http://www.horestco.com and you can also order it to your liking. -- View this message in contex

Re: [PATCH 54/68] drop strcpy in favor of raw sha1_to_hex

2015-09-24 Thread Jeff King
On Thu, Sep 24, 2015 at 07:42:05PM -0400, Eric Sunshine wrote: > On Thu, Sep 24, 2015 at 5:08 PM, Jeff King wrote: > > In some cases where we strcpy() the result of sha1_to_hex(), > > there's no need; the result goes directly into a printf > > statement, and we can simply pass the return value fr

Re: [PATCH 4/4] gc: remove broken refs

2015-09-24 Thread Jeff King
On Thu, Sep 24, 2015 at 05:08:41PM -0700, Junio C Hamano wrote: > One thing I wondered was if we can reliably tell between a ref that > wanted to be a real ref that records a broken object name and a ref > that wanted to be a symbolic ref that points a bogus thing, and if > we can't, should we wor

Re: [PATCH 1/2] SQUASH???

2015-09-24 Thread Junio C Hamano
Junio C Hamano writes: > I think that is a sensible change. Don't we want the same for the > other failure handler, though. Capture any message from it and > append it to the output of the process that just finished, or > something? Ah, that is already done. Scratch that "don't we want" part.

Re: [PATCH 0/2] Another squash on run-command: add an asynchronous parallel child processor

2015-09-24 Thread Junio C Hamano
Stefan Beller writes: > * If you do not die() in start_failure_fn or return_value_fn, you >don't want to write to stderr directly as you would destroy the fine >ordering of the processes output. So make the err strbuf available in >both these functions, and make sure the strbuf is ap

Re: [PATCH 1/2] SQUASH???

2015-09-24 Thread Junio C Hamano
Stefan Beller writes: > * If you do not die() in start_failure_fn or return_value_fn, you >don't want to write to stderr directly as you would destroy the fine >ordering of the processes output. So make the err strbuf available in >both these functions, and make sure the strbuf is ap

Re: [RFC/PATCH v1] Add Travis CI support

2015-09-24 Thread Junio C Hamano
larsxschnei...@gmail.com writes: > In order to avoid that in the future I configured Travis CI for Git. With this > patch Travis can run all Git tests including the "git-p4" and "Git-LFS" tests. Interesting. I was wondering about the "p4" part myself. > My idea is that the owner of "https://git

[PATCH] t/perf: make runner work even if Git is not installed

2015-09-24 Thread Stephan Beyer
aggregate.perl did not work when Git.pm is not installed to a directory contained in the default Perl library path list or PERLLIB. This commit prepends the Perl library path of the current Git source tree to enable this. Note that this commit adds a hard-coded relative path use lib '../../perl

Re: [PATCH 1/1] configure: make curl-config path configurable

2015-09-24 Thread Junio C Hamano
Remi Pommarel writes: > There are situations, ie during cross compilation, where curl-config program > is not present in the PATH. > > Make configure check that a custom curl-config program is passed by the user > through ac_cv_prog_CURL_CONFIG then set CURL_CONFIG variable accordingly in > confi

Re: [PATCH v2 1/1] Makefile: link libcurl before openssl and crypto

2015-09-24 Thread Junio C Hamano
Remi Pommarel writes: > For static linking especially library order while linking is important. For > example libssl contains symbol from libcrypto so the former should be linked > before the latter. That example is not a very good one to use for two reasons: (1) NEEDS_SSL_WITH_CRYPTO is a con

Re: [PATCH 4/4] gc: remove broken refs

2015-09-24 Thread Junio C Hamano
Jeff King writes: > For the same reasons as in my earlier responses, I think it's dangerous > to remove broken refs (it makes a small corruption much worse). It seems > reasonably sane to remove a dangling symref, though if we teach > for_each_ref to gracefully skip them, then leaving them in pla

Re: [PATCH 54/68] drop strcpy in favor of raw sha1_to_hex

2015-09-24 Thread Eric Sunshine
On Thu, Sep 24, 2015 at 5:08 PM, Jeff King wrote: > In some cases where we strcpy() the result of sha1_to_hex(), > there's no need; the result goes directly into a printf > statement, and we can simply pass the return value from > sha1_to_hex() directly. > > When this code was originally written,

[PATCH] t5561: get rid of racy appending to logfile

2015-09-24 Thread Stephan Beyer
The definition of log_div() appended information to the web server's logfile to make the test more readable. However, log_div() was called right after a request is served (which is done by git-http-backend); the web server waits for the git-http-backend process to exit before it writes to the log f

[RFC/PATCH v1] Add Travis CI support

2015-09-24 Thread larsxschneider
From: Lars Schneider The tests are executed on "Ubuntu 12.04 LTS Server Edition 64 bit" and on "OS X Mavericks" using gcc and clang. Perforce and Git-LFS are installed and therefore available for the respective tests. Signed-off-by: Lars Schneider --- .travis.yml | 28

[RFC/PATCH v1] Add Travis CI support

2015-09-24 Thread larsxschneider
From: Lars Schneider Hi, I recently broke a few tests... In order to avoid that in the future I configured Travis CI for Git. With this patch Travis can run all Git tests including the "git-p4" and "Git-LFS" tests. The tests are executed on "Ubuntu 12.04 LTS Server Edition 64 bit" and on "OS X

[PATCH 1/2] SQUASH???

2015-09-24 Thread Stefan Beller
This is a mixed bag of squashes. * pp_start_one() stuffed unborn child to the list of children when start_command() failed and start_failure() did not die(); return to the caller without corrupting children[] list in this case. * make poll(2) timeout used in pp_buffer_stderr() configurabl

[PATCH 2/2] SQUASH for "fetch_populated_submodules: use new parallel job processing"

2015-09-24 Thread Stefan Beller
This fixes the function signature in the first user of the async run processor. Signed-off-by: Stefan Beller --- submodule.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/submodule.c b/submodule.c index d7c7a6e..2c4396b 100644 --- a/submodule.c +++ b/submodule.c @@ -6

[PATCH 0/2] Another squash on run-command: add an asynchronous parallel child processor

2015-09-24 Thread Stefan Beller
Here is another proposal for squashing. It's basically both squash proposals by Junio, plus another issues I think should be done right from the beginning. [added by sb:] * If you do not die() in start_failure_fn or return_value_fn, you don't want to write to stderr directly as you would dest

Re: diff not finding difference

2015-09-24 Thread Jack Adrian Zappa
On Thu, Sep 24, 2015 at 5:09 PM, Jack Adrian Zappa wrote: > This is a weird one: > > [file-1 begin] > > abcd efg hijklmnop > > [file-1 end] > > [file-2 begin] > > blah blah blah > / > abdc boo ya! > > [file-2 end] > >

[PATCH 37/68] remote-ext: simplify git pkt-line generation

2015-09-24 Thread Jeff King
We format a pkt-line into a heap buffer, which requires manual computation of the required size, and uses some bare sprintf calls. We could use a strbuf instead, which would take care of the computation for us. But it's even easier still to use packet_write(). Besides handling the formatting and wr

Fwd: diff not finding difference

2015-09-24 Thread Jack Adrian Zappa
This is a weird one: [file-1 begin] abcd efg hijklmnop [file-1 end] [file-2 begin] blah blah blah / abdc boo ya! [file-2 end] Do a diff between these and it won't find any difference. Same with the following two

[PATCH 30/68] ref-filter: drop sprintf and strcpy calls

2015-09-24 Thread Jeff King
The ref-filter code comes from for-each-ref, and inherited a number of raw sprintf and strcpy calls. These are generally all safe, as we custom-size the buffers, or are formatting numbers into sufficiently large buffers. But we can make the resulting code even simpler and more obviously correct by

[PATCH 66/68] fsck: use for_each_loose_file_in_objdir

2015-09-24 Thread Jeff King
Since 27e1e22 (prune: factor out loose-object directory traversal, 2014-10-15), we now have a generic callback system for iterating over the loose object directories. This is used by prune, count-objects, etc. We did not convert git-fsck at the time because it implemented an inode-sorting scheme t

[PATCH 68/68] name-rev: use strip_suffix to avoid magic numbers

2015-09-24 Thread Jeff King
The manual size computations here are correct, but using strip_suffix makes that obvious, and hopefully communicates the intent of the code more clearly. Signed-off-by: Jeff King --- builtin/name-rev.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/builtin/name-rev.

[PATCH 52/68] daemon: use cld->env_array when re-spawning

2015-09-24 Thread Jeff King
This avoids an ugly strcat into a fixed-size buffer. It's not wrong (the buffer is plenty large enough for an IPv6 address plus some minor formatting), but it takes some effort to verify that. Unfortunately we are still stuck with some fixed-size buffers to hold the output of inet_ntop. But at lea

[PATCH 44/68] merge-recursive: convert malloc / strcpy to strbuf

2015-09-24 Thread Jeff King
This would be a fairly routine use of xstrfmt, except that we need to remember the length of the result to pass to cache_name_pos. So just use a strbuf, which makes this simple. As a bonus, this gets rid of confusing references to "pathlen+1". The "1" is for the trailing slash we added, but that i

[PATCH 41/68] init: use strbufs to store paths

2015-09-24 Thread Jeff King
The init code predates strbufs, and uses PATH_MAX-sized buffers along with many manual checks on intermediate sizes (some of which make magic assumptions, such as that init will not create a path inside .git longer than 50 characters). We can simplify this greatly by using strbufs, which drops som

[PATCH 64/68] fsck: drop inode-sorting code

2015-09-24 Thread Jeff King
Fsck tries to access loose objects in order of inode number, with the hope that this would make cold cache access faster on a spinning disk. This dates back to 7e8c174 (fsck-cache: sort entries by inode number, 2005-05-02), which predates the invention of packfiles. These days, there's not much po

[PATCH 34/68] read_remotes_file: simplify string handling

2015-09-24 Thread Jeff King
The main motivation for this cleanup is to switch our line-reading to a strbuf, which removes the use of a fixed-size buffer (which limited the size of remote URLs). Since we have the strbuf, we can make use of strbuf_rtrim(). While we're here, we can also simplify the parsing of each line. First

[PATCH 39/68] http-walker: store url in a strbuf

2015-09-24 Thread Jeff King
We do an unchecked sprintf directly into our url buffer. This doesn't overflow because we know that it was sized for "$base/objects/info/http-alternates", and we are writing "$base/objects/info/alternates", which must be smaller. But that is not immediately obvious to a reader who is looking for bu

[PATCH 47/68] write_loose_object: convert to strbuf

2015-09-24 Thread Jeff King
When creating a loose object tempfile, we use a fixed PATH_MAX-sized buffer, and strcpy directly into it. This isn't buggy, because we do a rough check of the size, but there's no verification that our guesstimate of the required space is enough (in fact, it's several bytes too big for the current

[PATCH 46/68] remove_leading_path: use a strbuf for internal storage

2015-09-24 Thread Jeff King
This function strcpy's directly into a PATH_MAX-sized buffer. There's only one caller, which feeds the git_dir into it, so it's not easy to trigger in practice (even if you fed a large $GIT_DIR through the environment or .git file, it would have to actually exist and be accessible on the filesystem

[PATCH 31/68] help: drop prepend function in favor of xstrfmt

2015-09-24 Thread Jeff King
This function predates xstrfmt, and its functionality is a subset. Let's just use xstrfmt. Signed-off-by: Jeff King --- builtin/help.c | 14 ++ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/builtin/help.c b/builtin/help.c index 3422e73..fba8c01 100644 --- a/builtin/h

[PATCH 36/68] upload-archive: convert sprintf to strbuf

2015-09-24 Thread Jeff King
When we report an error to the client, we format it into a fixed-size buffer using vsprintf(). This can't actually overflow in practice, since we only format a very tame subset of strings (mostly strerror() output). However, it's hard to tell immediately, so let's just use a strbuf so readers do no

[PATCH 67/68] use strbuf_complete to conditionally append slash

2015-09-24 Thread Jeff King
When working with paths in strbufs, we frequently want to ensure that a directory contains a trailing slash before appending to it. We can shorten this code (and make the intent more obvious) by calling strbuf_complete. Most of these cases are trivially identical conversions, but there are two thi

[PATCH 49/68] fetch-pack: use argv_array for index-pack / unpack-objects

2015-09-24 Thread Jeff King
This cleans up a magic number that must be kept in sync with the rest of the code (the number of argv slots). It also lets us drop some fixed buffers and an sprintf (since we can now use argv_array_pushf). We do still have to keep one fixed buffer for calling gethostname, but at least now the size

[PATCH 45/68] enter_repo: convert fixed-size buffers to strbufs

2015-09-24 Thread Jeff King
We use two PATH_MAX-sized buffers to represent the repo path, and must make sure not to overflow them. We do take care to check the lengths, but the logic is rather hard to follow, as we use several magic numbers (e.g., "PATH_MAX - 10"). And in fact you _can_ overflow the buffer if you have a ".git

[PATCH 65/68] Makefile: drop D_INO_IN_DIRENT build knob

2015-09-24 Thread Jeff King
Now that fsck has dropped its inode-sorting, there are no longer any users of this knob, and it can go away. Signed-off-by: Jeff King --- Makefile | 5 - config.mak.uname | 3 --- configure.ac | 7 --- 3 files changed, 15 deletions(-) diff --git a/Makefile b/Makefile index 8

[PATCH 55/68] color: add overflow checks for parsing colors

2015-09-24 Thread Jeff King
Our color parsing is designed to never exceed COLOR_MAXLEN bytes. But the relationship between that hand-computed number and the parsing code is not at all obvious, and we merely hope that it has been computed correctly for all cases. Let's mark the expected "end" pointer for the destination buffe

[PATCH 61/68] color: add color_set helper for copying raw colors

2015-09-24 Thread Jeff King
To set up default colors, we sometimes strcpy() from the default string literals into our color buffers. This isn't a bug (assuming the destination is COLOR_MAXLEN bytes), but makes it harder to audit the code for problematic strcpy calls. Let's introduce a color_set which copies under the assumpt

[PATCH 60/68] prefer memcpy to strcpy

2015-09-24 Thread Jeff King
When we already know the length of a string (e.g., because we just malloc'd to fit it), it's nicer to use memcpy than strcpy, as it makes it more obvious that we are not going to overflow the buffer (because the size we pass matches the size in the allocation). This also eliminates calls to strcpy

[PATCH 24/68] http-push: replace strcat with xsnprintf

2015-09-24 Thread Jeff King
We account for these strcats in our initial allocation, but the code is confusing to follow and verify. Let's remember our original allocation length, and then xsnprintf can verify that we don't exceed it. Note that we can't just use xstrfmt here (which would be even cleaner) because the code trie

[PATCH 59/68] help: clean up kfmclient munging

2015-09-24 Thread Jeff King
When we are going to launch "/path/to/konqueror", we instead rewrite this into "/path/to/kfmclient" by duplicating the original string and writing over the ending bits. This can be done more obviously with strip_suffix and xstrfmt. Note that we also fix a subtle bug with the "filename" parameter,

[PATCH 27/68] config: use xstrfmt in normalize_value

2015-09-24 Thread Jeff King
We xmalloc a fixed-size buffer and sprintf into it; this is OK because the size of our formatting types is finite, but that's not immediately clear to a reader auditing sprintf calls. Let's switch to xstrfmt, which is shorter and obviously correct. Note that just dropping the common xmalloc here c

[PATCH 28/68] fetch: replace static buffer with xstrfmt

2015-09-24 Thread Jeff King
We parse the INFINITE_DEPTH constant into a static, fixed-size buffer using sprintf. This buffer is sufficiently large for the current constant, but it's a suspicious pattern, as the constant is defined far away, and it's not immediately obvious that 12 bytes are large enough to hold it. We can ju

[PATCH 35/68] resolve_ref: use strbufs for internal buffers

2015-09-24 Thread Jeff King
resolve_ref already uses a strbuf internally when generating pathnames, but it uses fixed-size buffers for storing the refname and symbolic refs. This means that you cannot actually point HEAD to a ref that is larger than 256 bytes. We can lift this limit by using strbufs here, too. Like sb_path,

[PATCH 43/68] transport: use strbufs for status table "quickref" strings

2015-09-24 Thread Jeff King
We generate range strings like "1234abcd...5678efab" for use in the the fetch and push status tables. We use fixed-size buffers along with strcat to do so. These aren't buggy, as our manual size computation is correct, but there's nothing checking that this is so. Let's switch them to strbufs inst

[PATCH 51/68] stat_tracking_info: convert to argv_array

2015-09-24 Thread Jeff King
In addition to dropping the magic number for the fixed-size argv, we can also drop a fixed-length buffer and some strcpy's into it. Signed-off-by: Jeff King --- remote.c | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/remote.c b/remote.c index a01

[PATCH 50/68] http-push: use an argv_array for setup_revisions

2015-09-24 Thread Jeff King
This drops the magic number for the fixed-size argv arrays, so we do not have to wonder if we are overflowing it. We can also drop some confusing sha1_to_hex memory allocation (which seems to predate the ring of buffers allowing multiple calls), and get rid of an unchecked sprintf call. Signed-off

[PATCH 54/68] drop strcpy in favor of raw sha1_to_hex

2015-09-24 Thread Jeff King
In some cases where we strcpy() the result of sha1_to_hex(), there's no need; the result goes directly into a printf statement, and we can simply pass the return value from sha1_to_hex() directly. When this code was originally written, sha1_to_hex used a single buffer, and it was not safe to use i

[PATCH 40/68] sha1_get_pack_name: use a strbuf

2015-09-24 Thread Jeff King
We do some manual memory computation here, and there's no check that our 60 is not overflowed by the raw sprintf (it isn't, because the "which" parameter is never longer than "pack"). We can simplify this greatly with a strbuf. Technically the end result is not identical, as the original took care

[PATCH 53/68] use sha1_to_hex_r() instead of strcpy

2015-09-24 Thread Jeff King
Before sha1_to_hex_r() existed, a simple way to get hex sha1 into a buffer was with: strcpy(buf, sha1_to_hex(sha1)); This isn't wrong (assuming the buf is 41 characters), but it makes auditing the code base for bad strcpy() calls harder, as these become false positives. Let's convert them to s

[PATCH 38/68] http-push: use strbuf instead of fwrite_buffer

2015-09-24 Thread Jeff King
The http-push code defines an fwrite_buffer function for use as a curl callback; it just writes to a strbuf. There's no reason we need to use it ourselves, as we know we have a strbuf. This lets us format directly into it, rather than dealing with an extra temporary buffer (which required manual le

[PATCH 62/68] notes: document length of fanout path with a constant

2015-09-24 Thread Jeff King
We know that a fanned-out sha1 in a notes tree cannot be more than "aa/bb/cc/...", and we have an assert() to confirm that. But let's factor out that length into a constant so we can be sure it is used consistently. And even though we assert() earlier, let's replace a strcpy with xsnprintf, so it i

[PATCH 32/68] mailmap: replace strcpy with xstrdup

2015-09-24 Thread Jeff King
We want to make a copy of a string without any leading whitespace. To do so, we allocate a buffer large enough to hold the original, skip past the whitespace, then copy that. It's much simpler to just allocate after we've skipped, in which case we can just copy the remainder of the string, leaving

[PATCH 48/68] diagnose_invalid_index_path: use strbuf to avoid strcpy/strcat

2015-09-24 Thread Jeff King
We dynamically allocate a buffer and then strcpy and strcat into it. This isn't buggy, but we'd prefer to avoid these suspicious functions. This would be a good candidate for converstion to xstrfmt, but we need to record the length for dealing with index entries. A strbuf handles that for us. Sig

[PATCH 57/68] avoid sprintf and strcpy with flex arrays

2015-09-24 Thread Jeff King
When we are allocating a struct with a FLEX_ARRAY member, we generally compute the size of the array and then sprintf or strcpy into it. Normally we could improve a dynamic allocation like this by using xstrfmt, but it doesn't work here; we have to account for the size of the rest of the struct. B

[PATCH 33/68] read_branches_file: simplify string handling

2015-09-24 Thread Jeff King
This function does a lot of manual string handling, and has some unnecessary limits. This patch cleans up a number of things: 1. Drop the arbitrary 1000-byte limit on the size of the remote name (we do not have such a limit in any of the other remote-reading mechanisms). 2. Replace

[PATCH 29/68] use strip_suffix and xstrfmt to replace suffix

2015-09-24 Thread Jeff King
When we want to convert "foo.pack" to "foo.idx", we do it by duplicating the original string and then munging the bytes in place. Let's use strip_suffix and xstrfmt instead, which has several advantages: 1. It's more clear what the intent is. 2. It does not implicitly rely on the fact that

[PATCH 58/68] receive-pack: simplify keep_arg computation

2015-09-24 Thread Jeff King
To generate "--keep=receive-pack $pid on $host", we write progressively into a single buffer, which requires keeping track of how much we've written so far. But since the result is destined to go into our argv array, we can simply use argv_array_pushf. Unfortunately we still have to have a fixed-s

[PATCH 56/68] use alloc_ref rather than hand-allocating "struct ref"

2015-09-24 Thread Jeff King
This saves us some manual computation, and eliminates a call to strcpy. Signed-off-by: Jeff King --- builtin/fetch.c | 3 +-- remote-curl.c | 5 + 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 841880e..ed84963 100644 --- a/builtin/fe

[PATCH 42/68] apply: convert root string to strbuf

2015-09-24 Thread Jeff King
We use manual computation and strcpy to allocate the "root" variable. This would be much simpler using xstrfmt. But since we store the length, too, we can just use a strbuf, which handles that for us. Note that we stop distinguishing between "no root" and "empty root" in some cases, but that's OK

[PATCH 63/68] convert strncpy to memcpy

2015-09-24 Thread Jeff King
strncpy is known to be a confusing function because of its termination semantics. These calls are all correct, but it takes some examination to see why. In particular, every one of them expects to copy up to the length limit, and then makes some arrangement for terminating the result. We can just

[PATCH 09/68] fsck: use strbuf to generate alternate directories

2015-09-24 Thread Jeff King
When fsck-ing alternates, we make a copy of the alternate directory in a fixed PATH_MAX buffer. We memcpy directly, without any check whether we are overflowing the buffer. This is OK if PATH_MAX is a true representation of the maximum path on the system, because any path here will have already bee

[PATCH 12/68] progress: store throughput display in a strbuf

2015-09-24 Thread Jeff King
Coverity noticed that we strncpy() into a fixed-size buffer without making sure that it actually ended up NUL-terminated. This is unlikely to be a bug in practice, since throughput strings rarely hit 32 characters, but it would be nice to clean it up. The most obvious way to do so is to add a NUL-

[PATCH 08/68] add reentrant variants of sha1_to_hex and find_unique_abbrev

2015-09-24 Thread Jeff King
The sha1_to_hex and find_unique_abbrev functions always write into reusable static buffers. There are a few problems with this: - future calls overwrite our result. This is especially annoying with find_unique_abbrev, which does not have a ring of buffers, so you cannot even printf() a r

[PATCH 11/68] trace: use strbuf for quote_crnl output

2015-09-24 Thread Jeff King
When we output GIT_TRACE_SETUP paths, we quote any meta-characters. But our buffer to hold the result is only PATH_MAX bytes, and we could double the size of the input path (if every character needs quoting). We could use a 2*PATH_MAX buffer, if we assume the input will never be more than PATH_MAX.

[PATCH 25/68] receive-pack: convert strncpy to xsnprintf

2015-09-24 Thread Jeff King
This strncpy is pointless; we pass the strlen() of the src string, meaning that it works just like a memcpy. Worse, though, is that the size has no relation to the destination buffer, meaning it is a potential overflow. In practice, it's not. We pass only short constant strings like "warning: " an

[PATCH 22/68] entry.c: convert strcpy to xsnprintf

2015-09-24 Thread Jeff King
This particular conversion is non-obvious, because nobody has passed our function the length of the destination buffer. However, the interface to checkout_entry specifies that the buffer must be at least TEMPORARY_FILENAME_LENGTH bytes long, so we can check that (meaning the existing code was not b

[PATCH 19/68] stop_progress_msg: convert sprintf to xsnprintf

2015-09-24 Thread Jeff King
The usual arguments for using xsnprintf over sprintf apply, but this case is a little tricky. We print to a fixed-size buffer if we have room, and otherwise to an allocated buffer. So there should be no overflow here, but it is still good to communicate our intention, as well as to check our earlie

[PATCH 15/68] convert trivial sprintf / strcpy calls to xsnprintf

2015-09-24 Thread Jeff King
We sometimes sprintf into fixed-size buffers when we know that the buffer is large enough to fit the input (either because it's a constant, or because it's numeric input that is bounded in size). Likewise with strcpy of constant strings. However, these sites make it hard to audit sprintf and strcp

[PATCH 21/68] grep: use xsnprintf to format failure message

2015-09-24 Thread Jeff King
This looks at first glance like the sprintf can overflow our buffer, but it's actually fine; the p->origin string is something constant and small, like "command line" or "-e option". Signed-off-by: Jeff King --- grep.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grep.

[PATCH 20/68] compat/hstrerror: convert sprintf to snprintf

2015-09-24 Thread Jeff King
This is a trivially correct use of sprintf, as our error number should not be excessively long. But it's still nice to drop an sprintf call. Note that we cannot use xsnprintf here, because this is compat code which does not load git-compat-util.h. Signed-off-by: Jeff King --- compat/hstrerror.c

[PATCH 13/68] test-dump-cache-tree: avoid overflow of cache-tree name

2015-09-24 Thread Jeff King
When dumping a cache-tree, we sprintf sub-tree names directly into a fixed-size buffer, which can overflow. We can trivially fix this by converting to xsnprintf to at least notice and die. This probably should handle arbitrary-sized names, but there's not much point. It's used only by the test scr

[PATCH 14/68] compat/inet_ntop: fix off-by-one in inet_ntop4

2015-09-24 Thread Jeff King
Our compat inet_ntop4 function writes to a temporary buffer with snprintf, and then uses strcpy to put the result into the final "dst" buffer. We check the return value of snprintf against the size of "dst", but fail to account for the NUL terminator. As a result, we may overflow "dst" with a singl

[PATCH 07/68] strbuf: make strbuf_complete_line more generic

2015-09-24 Thread Jeff King
The strbuf_complete_line function makes sure that a buffer ends in a newline. But we may want to do this for any character (e.g., "/" on the end of a path). Let's factor out a generic version, and keep strbuf_complete_line as a thin wrapper. Signed-off-by: Jeff King --- strbuf.h | 15 +++

[PATCH 26/68] replace trivial malloc + sprintf / strcpy calls with xstrfmt

2015-09-24 Thread Jeff King
It's a common pattern to do: foo = xmalloc(strlen(one) + strlen(two) + 1 + 1); sprintf(foo, "%s %s", one, two); (or possibly some variant with strcpy()s or a more complicated length computation). We can switch these to use xstrfmt, which is shorter, involves less error-prone manual computati

[PATCH 18/68] find_short_object_filename: convert sprintf to xsnprintf

2015-09-24 Thread Jeff King
We use sprintf() to format some hex data into a buffer. The buffer is clearly long enough, and using snprintf here is not necessary. And in fact, it does not really make anything easier to audit, as the size we feed to snprintf accounts for the magic extra 42 bytes found in each alt->name field of

[PATCH 23/68] add_packed_git: convert strcpy into xsnprintf

2015-09-24 Thread Jeff King
We have the path "foo.idx", and we create a buffer big enough to hold "foo.pack" and "foo.keep", and then strcpy straight into it. This isn't a bug (we have enough space), but it's very hard to tell from the strcpy that this is so. Let's instead use strip_suffix to take off the ".idx", record the

[PATCH 10/68] mailsplit: make PATH_MAX buffers dynamic

2015-09-24 Thread Jeff King
There are several PATH_MAX-sized buffers in mailsplit, along with some questionable uses of sprintf. These are not really of security interest, as local mailsplit pathnames are not typically under control of an attacker, and you could generally only overflow a few numbers at the end of a path that

[PATCH 05/68] add xsnprintf helper function

2015-09-24 Thread Jeff King
There are a number of places in the code where we call sprintf(), with the assumption that the output will fit into the buffer. In many cases this is true (e.g., formatting a number into a large buffer), but it is hard to tell immediately from looking at the code. It would be nice if we had some ru

[PATCH 06/68] add git_path_buf helper function

2015-09-24 Thread Jeff King
If you have a function that uses git_path a lot, but would prefer to avoid the static buffers, it's useful to keep a single scratch buffer locally and reuse it for each call. You used to be able to do this with git_snpath: char buf[PATH_MAX]; foo(git_snpath(buf, sizeof(buf), "foo")); bar(gi

[PATCH 17/68] use xsnprintf for generating git object headers

2015-09-24 Thread Jeff King
We generally use 32-byte buffers to format git's "type size" header fields. These should not generally overflow unless you can produce some truly gigantic objects (and our types come from our internal array of constant strings). But it is a good idea to use xsnprintf to make sure this is the case.

[PATCH 16/68] archive-tar: use xsnprintf for trivial formatting

2015-09-24 Thread Jeff King
When we generate tar headers, we sprintf() values directly into a struct with the fixed-size header values. For the most part this is fine, as we are formatting small values (e.g., the octal format of "mode & 0x" is of fixed length). But it's still a good idea to use xsnprintf here. It communic

[PATCH 04/68] fsck: don't fsck alternates for connectivity-only check

2015-09-24 Thread Jeff King
Commit 02976bf (fsck: introduce `git fsck --connectivity-only`, 2015-06-22) recently gave fsck an option to perform only a subset of the checks, by skipping the fsck_object_dir() call. However, it does so only for the local object directory, and we still do expensive checks on any alternate repos.

[PATCH 03/68] archive-tar: fix minor indentation violation

2015-09-24 Thread Jeff King
This looks like a simple omission from 8539070 (archive-tar: unindent write_tar_entry by one level, 2012-05-03). Signed-off-by: Jeff King --- archive-tar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archive-tar.c b/archive-tar.c index 0d1e6bd..b6b30bb 100644 --- a/archiv

[PATCH 01/68] show-branch: avoid segfault with --reflog of unborn branch

2015-09-24 Thread Jeff King
When no branch is given to the "--reflog" option, we resolve HEAD to get the default branch. However, if HEAD points to an unborn branch, resolve_ref returns NULL, and we later segfault trying to access it. Signed-off-by: Jeff King --- builtin/show-branch.c | 2 ++ 1 file changed, 2 insertions(+

[PATCH 02/68] mailsplit: fix FILE* leak in split_maildir

2015-09-24 Thread Jeff King
If we encounter an error while splitting a maildir, we exit the function early, leaking the open filehandle. This isn't a big deal, since we exit the program soon after, but it's easy enough to be careful. Signed-off-by: Jeff King --- builtin/mailsplit.c | 5 - 1 file changed, 4 insertions(+

[PATCH v2 0/68] war on sprintf

2015-09-24 Thread Jeff King
This is a revised version of the series I sent earlier[1]. For those just joining us, the goal is to remove sprintf, strcpy, etc, to make it easier to audit the code base for buffer overflows. I've been addressing review comments for individual patches as the discussion progressed, so there's noth

[PATCH 1/1] configure: make curl-config path configurable

2015-09-24 Thread Remi Pommarel
There are situations, ie during cross compilation, where curl-config program is not present in the PATH. Make configure check that a custom curl-config program is passed by the user through ac_cv_prog_CURL_CONFIG then set CURL_CONFIG variable accordingly in config.mak.autogen. Makefile uses this v

Re: [PATCH v7 0/7] git-p4: add support for large file systems

2015-09-24 Thread Luke Diamand
On 23 September 2015 at 12:42, Lars Schneider wrote: > > On 23 Sep 2015, at 13:25, Luke Diamand wrote: > >> Adding back git@vger.kernel.org, which I inadvertently dropped off the >> thread. >> >> On 23 September 2015 at 12:22, Luke Diamand wrote: >>> On 23 September 2015 at 11:09, Lars Schneide

Re: git-p4: t9819 failing

2015-09-24 Thread Luke Diamand
OK, slight correction there - it now doesn't crash getting the disk usage, but I think it still needs to be updated following the other changes to case-handling. Luke On 24 September 2015 at 08:45, Luke Diamand wrote: > On 23 September 2015 at 13:28, Lars Schneider > wrote: >> >>> Here's the l

Business Deal

2015-09-24 Thread William Leung
I have a business deal for you worth 24.5m usd, Reply: wl548...@gmail.com for more details if interested. Thank You --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus -- To unsubscribe from this list: send the line "unsubscribe git" in the bo

[PATCH 1/1] configure.ac: detect ssl need with libcurl

2015-09-24 Thread Remi Pommarel
When libcurl has been statically compiled with openssl support they both need to be linked in everytime libcurl is used. During configuration this can be detected by looking for Curl_ssl_init function symbol in libcurl, which will only be present if libcurl has been compiled statically built with

Re: [PATCH] t5561: get rid of racy appending to logfile

2015-09-24 Thread Jeff King
On Thu, Sep 24, 2015 at 08:12:22PM +0200, Stephan Beyer wrote: > The definition of log_div() appended information to the web server's > logfile to make the test more readable. However, log_div() was called > right after a request is served (which is done by git-http-backend); > the web server wait

[PATCH] t5561: get rid of racy appending to logfile

2015-09-24 Thread Stephan Beyer
The definition of log_div() appended information to the web server's logfile to make the test more readable. However, log_div() was called right after a request is served (which is done by git-http-backend); the web server waits for the git-http-backend process to exit before it writes to the log f

[PATCH v6b 5/8] branch: drop non-commit error reporting

2015-09-24 Thread Karthik Nayak
Remove the error "branch '%s' does not point at a commit" in apppend_ref() which reports branch refs which do not point to commits. Also remove the error "some refs could not be read" in print_ref_list() which is triggered as a consequence of the first error. This seems to be the wrong codepath wh

Re: [PATCH 4/4] gc: remove broken refs

2015-09-24 Thread Jeff King
On Thu, Sep 24, 2015 at 11:14:02AM +0200, Johannes Schindelin wrote: > When encountering broken refs, such as a stale remote HEAD (which can > happen if the active branch was renamed in the remote), it is more > helpful to remove those refs than to exit with an error. For the same reasons as in m

  1   2   >