Re: [PULL] svn pathnameencoding for git svn dcommit

2016-02-15 Thread Eric Wong
Junio, sorry about basing on next, I somehow thought I was going to need to depend on something there. Updated pull below if Kazutoshi can run the test effectively. Kazutoshi Satoda wrote: > Thank you for the tests. But, on my environment, both of them failed >

Re: [PATCH 14/18] git-compat-util: drop mempcpy compat code

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 4:56 PM, Jeff King wrote: > There are no callers of this left, as the last one was > dropped in the previous patch. And there are no likely to be s/no/not > new ones, as the function has been around since 2010 without > gaining any new callers. > >

Re: [PATCH 13/18] sequencer: simplify memory allocation of get_message

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 4:56 PM, Jeff King wrote: > For a commit with has "1234abcd" and subject "foo", this Did you mean s/with has/which has ID/ or something? > function produces a struct with three strings: > > 1. "foo" > > 2. "1234abcd... foo" > > 3. "parent of 1234abcd...

Re: [PATCH] merge_blobs: use strbuf instead of manually-sized mmfile_t

2016-02-15 Thread Jeff King
On Tue, Feb 16, 2016 at 12:09:15AM -0500, Eric Sunshine wrote: > > - ecb.priv = res; > > - return xdi_diff(f1, f2, , , ); > > + res->size = out.len; /* avoid long/size_t pointer mismatch below */ > > It took a minute or two for me to realize that "mismatch below" was > talking about the

Re: [PATCH 08/18] use st_add and st_mult for allocation size computation

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 4:53 PM, Jeff King wrote: > If our size computation overflows size_t, we may allocate a > much smaller buffer than we expected and overflow it. It's > probably impossible to trigger an overflow in most of these > sites in practice, but it is easy enough

Re: [PATCH 05/18] convert trivial cases to ALLOC_ARRAY

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 11:32:25PM -0500, Eric Sunshine wrote: > On Mon, Feb 15, 2016 at 11:23 PM, Jeff King wrote: > > On Mon, Feb 15, 2016 at 11:22:12PM -0500, Eric Sunshine wrote: > >> On Mon, Feb 15, 2016 at 4:51 PM, Jeff King wrote: > >> > - path =

Re: [PATCH] merge_blobs: use strbuf instead of manually-sized mmfile_t

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 08:12:58PM -0500, Jeff King wrote: > On Mon, Feb 15, 2016 at 10:39:39PM +0100, Stefan Frühwirth wrote: > > in one specific circumstance, git-merge-tree exits with a segfault caused by > > "*** Error in `git': malloc(): memory corruption (fast)": > > > > There is a test

Re: [PATCH 05/18] convert trivial cases to ALLOC_ARRAY

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 11:23 PM, Jeff King wrote: > On Mon, Feb 15, 2016 at 11:22:12PM -0500, Eric Sunshine wrote: >> On Mon, Feb 15, 2016 at 4:51 PM, Jeff King wrote: >> > - path = xmalloc((n+1)*sizeof(char *)); >> > + ALLOC_ARRAY(path, n+1); >> >>

Re: [PATCH 05/18] convert trivial cases to ALLOC_ARRAY

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 11:22:12PM -0500, Eric Sunshine wrote: > On Mon, Feb 15, 2016 at 4:51 PM, Jeff King wrote: > > Each of these cases can be converted to use ALLOC_ARRAY or > > REALLOC_ARRAY, which has two advantages: > > > > 1. It automatically checks the array-size

Re: [PATCH 07/18] convert trivial cases to FLEX_ARRAY macros

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 11:18:56PM -0500, Eric Sunshine wrote: > > diff --git a/builtin/reflog.c b/builtin/reflog.c > > @@ -408,13 +407,12 @@ static struct reflog_expire_cfg *find_cfg_ent(const > > char *pattern, size_t len) > > reflog_expire_cfg_tail = _expire_cfg; > > > >

Re: [PATCH 05/18] convert trivial cases to ALLOC_ARRAY

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 4:51 PM, Jeff King wrote: > Each of these cases can be converted to use ALLOC_ARRAY or > REALLOC_ARRAY, which has two advantages: > > 1. It automatically checks the array-size multiplication > for overflow. > > 2. It always uses sizeof(*array) for

Re: [PATCH 07/18] convert trivial cases to FLEX_ARRAY macros

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 10:36 PM, Jeff King wrote: > On Mon, Feb 15, 2016 at 10:26:26PM -0500, Jeff King wrote: >> We could do this on top of my series (I can also factor out the fix >> separately to go at the beginning if we don't want to hold the bugfix >> hostage). >> >> -- >8

Re: [PATCH 07/18] convert trivial cases to FLEX_ARRAY macros

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 10:15 PM, Jeff King wrote: > On Mon, Feb 15, 2016 at 09:17:05PM -0500, Eric Sunshine wrote: >> > - ent = xcalloc(1, (sizeof(*ent) + len)); >> > - memcpy(ent->pattern, pattern, len); >> > + FLEX_ALLOC_MEM(ent, pattern, pattern, len); >> >>

Re: [PATCH 07/18] convert trivial cases to FLEX_ARRAY macros

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 10:26:26PM -0500, Jeff King wrote: > We could do this on top of my series (I can also factor out the fix > separately to go at the beginning if we don't want to hold the bugfix > hostage). > > -- >8 -- > Subject: [PATCH] reflog_expire_cfg: drop misleading "len" parameter

Re: [PULL] svn pathnameencoding for git svn dcommit

2016-02-15 Thread Kazutoshi Satoda
On 2016/02/15 9:52 +0900, Eric Wong wrote: > I've amended tests to both commits, but the URL encoding one > requires an HTTP server to test effectively. Thank you for the tests. But, on my environment, both of them failed unexpectedly. (Windows 7 SP1, x86_64 Cygwin, LANG=ja_JP.UTF-8) For now, I

Re: [PATCH +warn] Implement https public key pinning

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 07:19:07PM -0800, Junio C Hamano wrote: > I suspect that "#else" is too agressive to bail out or something > silly like that. > > Oh, I think I found it. > > @@ -216,6 +219,13 @@ static int http_options(const char *var, const char > *value, void *cb) > if

Re: [PATCH 07/18] convert trivial cases to FLEX_ARRAY macros

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 10:15:54PM -0500, Jeff King wrote: > > Answering my own question: Looking at reflog_expire_config() and > > parse_config_key(), I gather that 'len' already accounts for the NUL, > > thus the new code is overallocating (which should not be a problem). > > Actually, I think

Re: [PATCH +warn] Implement https public key pinning

2016-02-15 Thread Junio C Hamano
Jeff King writes: > On Mon, Feb 15, 2016 at 03:25:32PM -0800, Junio C Hamano wrote: > >> Thanks. This, when applied on top of 2.7.1, however seems to break >> at least t5541 and t5551. > > Hrm. I cannot see how the new code can possibly do anything unless > http.pinnedpubkey is

Re: [PATCH 07/18] convert trivial cases to FLEX_ARRAY macros

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 09:17:05PM -0500, Eric Sunshine wrote: > > --- > > diff --git a/builtin/reflog.c b/builtin/reflog.c > > @@ -412,8 +410,7 @@ static struct reflog_expire_cfg *find_cfg_ent(const > > char *pattern, size_t len) > > !memcmp(ent->pattern, pattern, len)) > >

Re: [PATCH 04/18] add helpers for allocating flex-array structs

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 08:47:30PM -0500, Eric Sunshine wrote: > > This patch adds a few helpers to turn simple cases of into a > > Grammo: "cases of into" Oops. Cases of "flex-array struct allocation into...". > > + * and "name" will point to a block of memory after the struct, which will >

[PATCH] wt-status.c: set commitable bit if there is a meaningful merge.

2016-02-15 Thread Stephen P. Smith
The 'commit --dry-run' and commit return values differed if a conflicted merge had been resolved and the commit would be the same as the parent. Update show_merge_in_progress to set the commitable bit if conflicts have been resolved and a merge is in progress. Signed-off-by: Stephen P. Smith

Re: [PATCH 07/18] convert trivial cases to FLEX_ARRAY macros

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 4:52 PM, Jeff King wrote: > Using FLEX_ARRAY macros reduces the amount of manual > computation size we have to do. It also ensures we don't > overflow size_t, and it makes sure we write the same number > of bytes that we allocated. > > Signed-off-by: Jeff

Re: [PATCH 04/18] add helpers for allocating flex-array structs

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 4:50 PM, Jeff King wrote: > Allocating a struct with a flex array is pretty simple in > practice: you over-allocate the struct, then copy some data > into the over-allocation. But it can be a slight pain to > make sure you're allocating and copying the right

Re: [PATCH 3/4] dir.c: support marking some patterns already matched

2016-02-15 Thread Duy Nguyen
On Tue, Feb 16, 2016 at 6:47 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> Given path "a" and the pattern "a", it's matched. But if we throw path >> "a/b" to pattern "a", the code fails to realize that if "a" matches >> "a" then "a/b" should

Git Submodule auto-update

2016-02-15 Thread Cameron W
I apologize if this is a dumb or repeated question. Is there a way to have a submodule automatically 'update' on pull of the parent repository, WITHOUT requiring each user/committer to change any settings (hooks or git config aliases)? Ideally, a setting I can change at the repository level

Re: [PATCH +warn] Implement https public key pinning

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 03:25:32PM -0800, Junio C Hamano wrote: > Thanks. This, when applied on top of 2.7.1, however seems to break > at least t5541 and t5551. Hrm. I cannot see how the new code can possibly do anything unless http.pinnedpubkey is set, and our tests don't do that. Neither

Re: [PATCH 1/4] dir.c: fix match_pathname()

2016-02-15 Thread Duy Nguyen
On Tue, Feb 16, 2016 at 6:29 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> Given the pattern "1/2/3/4" and the path "1/2/3/4/f", the pattern >> prefix is "1/2/3/4". We will compare and remove the prefix from both >> pattern and path and come

Re: [PATCH 1/2] worktree: fix "add -B"

2016-02-15 Thread Duy Nguyen
On Tue, Feb 16, 2016 at 6:53 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> Current code does not update "symref" when -B is used. This string >> contains the new HEAD. Because it's empty "git worktree add -B" fails at >> symbolic-ref step.

[PATCH] merge_blobs: use strbuf instead of manually-sized mmfile_t

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 10:39:39PM +0100, Stefan Frühwirth wrote: > in one specific circumstance, git-merge-tree exits with a segfault caused by > "*** Error in `git': malloc(): memory corruption (fast)": > > There has to be at least one commit first (as far as I can tell it doesn't > matter

Re: Custom merge driver with no rename detection

2016-02-15 Thread Felipe Gonçalves Assis
On 15 February 2016 at 09:03, Junio C Hamano wrote: > Felipe Gonçalves Assis writes: > >> However, if you do find this approach acceptable/desirable >> (rename-threshold > 100%), I can work on the issues pointed out and >> propose a proper patch. > >

[PATCH] merge-recursive: option to disable renames

2016-02-15 Thread Felipe Gonçalves Assis
The recursive strategy turns on rename detection by default. Add a strategy option to disable rename detection even for exact renames. Signed-off-by: Felipe Gonçalves Assis --- Hi, this is a patch relative to the "Custom merge driver with no rename detection" thread,

Re: [PATCH v2 4/4] remote: use remote_is_configured() for add and rename

2016-02-15 Thread Thomas Gummerer
On 02/15, Jeff King wrote: > On Mon, Feb 15, 2016 at 05:52:14PM -0500, Eric Sunshine wrote: > > > > diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh > > > @@ -157,6 +157,24 @@ test_expect_success 'rename errors out early when > > > deleting non-existent branch' > > > +test_expect_success 'add

Re: [PATCH v2 1/4] remote: use parse_config_key

2016-02-15 Thread Thomas Gummerer
On 02/15, Jeff King wrote: > On Mon, Feb 15, 2016 at 11:39:41PM +0100, Thomas Gummerer wrote: > > > - if (!starts_with(key, "remote.")) > > + if (parse_config_key(key, "remote", , , ) < 0) > > return 0; > > - name = key + 7; > > > > /* Handle remote.* variables */ > > - if

Re: [PATCH 1/2] worktree: fix "add -B"

2016-02-15 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy writes: > Current code does not update "symref" when -B is used. This string > contains the new HEAD. Because it's empty "git worktree add -B" fails at > symbolic-ref step. > > Because branch creation is already done before calling add_worktree(), > -B is

Re: [PATCH 0/4] .gitignore, reinclude rules, take 2

2016-02-15 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy writes: > Take one was bad and reverted in commit 8c72236. Take two provides a > more complete solution to the pair of rules > > exclude/this > !exclude/this/except/this > > 3/4 should do a better job at stopping regressions in take 1. 4/4 > provides

Re: [PATCH 3/4] dir.c: support marking some patterns already matched

2016-02-15 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy writes: > Given path "a" and the pattern "a", it's matched. But if we throw path > "a/b" to pattern "a", the code fails to realize that if "a" matches > "a" then "a/b" should also be matched. > > When the pattern is matched the first time, we can mark it

Re: [PATCH 1/4] dir.c: fix match_pathname()

2016-02-15 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy writes: > Given the pattern "1/2/3/4" and the path "1/2/3/4/f", the pattern > prefix is "1/2/3/4". We will compare and remove the prefix from both > pattern and path and come to this code > > /* >* If the whole pattern did not have a

Re: [PATCH +warn] Implement https public key pinning

2016-02-15 Thread Junio C Hamano
Thanks. This, when applied on top of 2.7.1, however seems to break at least t5541 and t5551. -- 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 4/4] remote: use remote_is_configured() for add and rename

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 05:52:14PM -0500, Eric Sunshine wrote: > > diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh > > @@ -157,6 +157,24 @@ test_expect_success 'rename errors out early when > > deleting non-existent branch' > > +test_expect_success 'add existing foreign_vcs remote' ' > > +

Re: [PATCH v2 1/4] remote: use parse_config_key

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 11:39:41PM +0100, Thomas Gummerer wrote: > - if (!starts_with(key, "remote.")) > + if (parse_config_key(key, "remote", , , ) < 0) > return 0; > - name = key + 7; > > /* Handle remote.* variables */ > - if (!strcmp(name,

Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 02:18:18PM -0800, Junio C Hamano wrote: > larsxschnei...@gmail.com writes: > > > +test_expect_success 'set up custom config file' ' > > + CUSTOM_CONFIG_FILE=$(printf "file\twith\ttabs.conf") && > > + cat >"$CUSTOM_CONFIG_FILE" <<-\EOF > > + [user] > > +

Re: [PATCH v2 4/4] remote: use remote_is_configured() for add and rename

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 5:39 PM, Thomas Gummerer wrote: > Both remote add and remote rename use a slightly different hand-rolled > check if the remote exits. The hand-rolled check may have some subtle > cases in which it might fail to detect when a remote already exists. >

[PATCH v2 1/4] remote: use parse_config_key

2016-02-15 Thread Thomas Gummerer
95b567c7 ("use skip_prefix to avoid repeating strings") transformed calls using starts_with() and then skipping the length of the prefix to skip_prefix() calls. In remote.c there are a few calls like: if (starts_with(foo, "bar")) foo += 3 These calls weren't touched by the aformentioned

[PATCH v2 3/4] remote: actually check if remote exits

2016-02-15 Thread Thomas Gummerer
When converting the git remote command to a builtin in 211c89 ("Make git-remote a builtin"), a few calls to check if a remote exists were converted from: if (!exists $remote->{$name}) { [...] to: remote = remote_get(argv[1]); if (!remote) [...] The new

[PATCH v2 4/4] remote: use remote_is_configured() for add and rename

2016-02-15 Thread Thomas Gummerer
Both remote add and remote rename use a slightly different hand-rolled check if the remote exits. The hand-rolled check may have some subtle cases in which it might fail to detect when a remote already exists. One such case was fixed in fb86e32 ("git remote: allow adding remotes agreeing with

[PATCH v2 2/4] remote: simplify remote_is_configured()

2016-02-15 Thread Thomas Gummerer
The remote_is_configured() function allows checking whether a remote exists or not. The function however only works if remote_get() wasn't called before calling it. In addition, it only checks the configuration for remotes, but not remotes or branches files. Make use of the origin member of

Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value

2016-02-15 Thread Ramsay Jones
On 15/02/16 21:40, Jeff King wrote: > On Mon, Feb 15, 2016 at 09:36:23PM +, Ramsay Jones wrote: > >>> +test_expect_success '--show-origin stdin' ' >>> + cat >expect <<-\EOF && >>> + stdin: user.custom=true >> >> So, as with the previous patch, I think this should be: >>

[PATCH v2 0/4] git remote improvements

2016-02-15 Thread Thomas Gummerer
Thanks Peff for the review on the previous round ($gmane/286214). The series now uses parse_config_key() instead of skip_prefix, and I added REMOTE_UNCONFIGURED to the enum used in the origin field in struct remote. Interdiff below: diff --git a/remote.c b/remote.c index 3a4ca9b..d10ae00 100644

Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value

2016-02-15 Thread Junio C Hamano
larsxschnei...@gmail.com writes: > +test_expect_success 'set up custom config file' ' > + CUSTOM_CONFIG_FILE=$(printf "file\twith\ttabs.conf") && > + cat >"$CUSTOM_CONFIG_FILE" <<-\EOF > + [user] > + custom = true > + EOF > +' > + > +test_expect_success

Re: [PATCH 0/18] hardening allocations against integer overflow

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 04:45:16PM -0500, Jeff King wrote: > The only bug I have actually confirmed in practice here is fixed by > patch 2 (which is why it's at the front). There's another one in > path_name(), but that function is already dropped by the nearby > jk/lose-name-path topic. > > The

Re: git svn dcommit doesn't support --username option for file:/// urls

2016-02-15 Thread Tim Ringenbach
On Mon, Feb 15, 2016 at 3:14 PM, Eric Wong wrote: > It might take a while for me to get around to looking at this > more, so it would be very helpful if you poked around and tried > some different things in the source. Ok, I played around with it some and found something

[PATCH 18/18] ewah: convert to REALLOC_ARRAY, etc

2016-02-15 Thread Jeff King
Now that we're built around xmalloc and friends, we can use helpers like REALLOC_ARRAY, ALLOC_GROW, and so on to make the code shorter and protect against integer overflow. Signed-off-by: Jeff King --- ewah/bitmap.c | 16 ewah/ewah_bitmap.c | 5 ++---

[PATCH 17/18] convert ewah/bitmap code to use xmalloc

2016-02-15 Thread Jeff King
This code was originally written with the idea that it could be spun off into its own ewah library, and uses the overrideable ewah_malloc to do allocations. We plug in xmalloc as our ewah_malloc, of course. But over the years the ewah code itself has become more entangled with git, and the return

[PATCH 16/18] diff_populate_gitlink: use a strbuf

2016-02-15 Thread Jeff King
We allocate 100 bytes to hold the "Submodule commit ..." text. This is enough, but it's not immediately obvious that this is the case, and we have to repeat the magic 100 twice. We could get away with xstrfmt here, but we want to know the size, as well, so let's use a real strbuf. And while we're

[PATCH 15/18] transport_anonymize_url: use xstrfmt

2016-02-15 Thread Jeff King
This function uses xcalloc and two memcpy calls to concatenate two strings. We can do this as an xstrfmt one-liner, and then it is more clear that we are allocating the correct amount of memory. Signed-off-by: Jeff King --- transport.c | 8 +++- 1 file changed, 3

[PATCH 14/18] git-compat-util: drop mempcpy compat code

2016-02-15 Thread Jeff King
There are no callers of this left, as the last one was dropped in the previous patch. And there are no likely to be new ones, as the function has been around since 2010 without gaining any new callers. Signed-off-by: Jeff King --- git-compat-util.h | 9 - 1 file changed,

[PATCH 13/18] sequencer: simplify memory allocation of get_message

2016-02-15 Thread Jeff King
For a commit with has "1234abcd" and subject "foo", this function produces a struct with three strings: 1. "foo" 2. "1234abcd... foo" 3. "parent of 1234abcd... foo" It takes advantage of the fact that these strings are subsets of each other, and allocates only _one_ string, with pointers

[PATCH 12/18] test-path-utils: fix normalize_path_copy output buffer size

2016-02-15 Thread Jeff King
The normalize_path_copy function needs an output buffer that is at least as long as its input (it may shrink the path, but never expand it). However, this test program feeds it static PATH_MAX-sized buffers, which have no relation to the input size. In the normalize_ceiling_entry case, we do at

Re: malloc memory corruption on merge-tree with leading newline

2016-02-15 Thread Stefan Frühwirth
Addendum: Problem occurs with version 2.7.1 as well as version 1.9.1. On 15/02/16 22:39, Stefan Frühwirth wrote: in one specific circumstance, git-merge-tree exits with a segfault caused by "*** Error in `git': malloc(): memory corruption (fast)": Stefan -- To unsubscribe from this list:

[PATCH 11/18] fetch-pack: simplify add_sought_entry

2016-02-15 Thread Jeff King
We have two variants of this function, one that takes a string and one that takes a ptr/len combo. But we only call the latter with the length of a NUL-terminated string, so our first simplification is to drop it in favor of the string variant. Since we know we have a string, we can also replace

[PATCH 10/18] fast-import: simplify allocation in start_packfile

2016-02-15 Thread Jeff King
This function allocates a packed_git flex-array, and adds a mysterious 2 bytes to the length of the pack_name field. One is for the trailing NUL, but the other has no purpose. This is probably cargo-culted from add_packed_git, which gets the ".idx" path and needs to allocate enough space to hold

[PATCH 09/18] write_untracked_extension: use FLEX_ALLOC helper

2016-02-15 Thread Jeff King
We perform unchecked additions when computing the size of a "struct ondisk_untracked_cache". This is unlikely to have an integer overflow in practice, but we'd like to avoid this dangerous pattern to make further audits easier. Note that there's one subtlety here, though. We protect ourselves

[PATCH 08/18] use st_add and st_mult for allocation size computation

2016-02-15 Thread Jeff King
If our size computation overflows size_t, we may allocate a much smaller buffer than we expected and overflow it. It's probably impossible to trigger an overflow in most of these sites in practice, but it is easy enough convert their additions and multiplications into overflow-checking variants.

[PATCH 07/18] convert trivial cases to FLEX_ARRAY macros

2016-02-15 Thread Jeff King
Using FLEX_ARRAY macros reduces the amount of manual computation size we have to do. It also ensures we don't overflow size_t, and it makes sure we write the same number of bytes that we allocated. Signed-off-by: Jeff King --- attr.c | 4 +--- builtin/blame.c

[PATCH 06/18] use xmallocz to avoid size arithmetic

2016-02-15 Thread Jeff King
We frequently allocate strings as xmalloc(len + 1), where the extra 1 is for the NUL terminator. This can be done more simply with xmallocz, which also checks for integer overflow. There's no case where switching xmalloc(n+1) to xmallocz(n) is wrong; the result is the same length, and malloc made

Re: [PATCH] http: add option to try authentication without username

2016-02-15 Thread brian m. carlson
On Mon, Feb 15, 2016 at 04:46:43PM -0500, Eric Sunshine wrote: > On Mon, Feb 15, 2016 at 4:39 PM, Junio C Hamano wrote: > > "brian m. carlson" writes: > >> On Mon, Feb 15, 2016 at 03:34:51PM -0500, Jeff King wrote: > >>> So I think this hack

[PATCH 05/18] convert trivial cases to ALLOC_ARRAY

2016-02-15 Thread Jeff King
Each of these cases can be converted to use ALLOC_ARRAY or REALLOC_ARRAY, which has two advantages: 1. It automatically checks the array-size multiplication for overflow. 2. It always uses sizeof(*array) for the element-size, so that it can never go out of sync with the declared

[PATCH 04/18] add helpers for allocating flex-array structs

2016-02-15 Thread Jeff King
Allocating a struct with a flex array is pretty simple in practice: you over-allocate the struct, then copy some data into the over-allocation. But it can be a slight pain to make sure you're allocating and copying the right amounts. This patch adds a few helpers to turn simple cases of into a

[PATCH 03/18] harden REALLOC_ARRAY and xcalloc against size_t overflow

2016-02-15 Thread Jeff King
REALLOC_ARRAY inherently involves a multiplication which can overflow size_t, resulting in a much smaller buffer than we think we've allocated. We can easily harden it by using st_mult() to check for overflow. Likewise, we can add ALLOC_ARRAY to do the same thing for xmalloc calls. xcalloc()

[PATCH 02/18] tree-diff: catch integer overflow in combine_diff_path allocation

2016-02-15 Thread Jeff King
A combine_diff_path struct has two "flex" members allocated alongside the struct: a string to hold the pathname, and an array of parent pointers. We use an "int" to compute this, meaning we may easily overflow it if the pathname is extremely long. We can fix this by using size_t, and checking for

[PATCH 01/18] add helpers for detecting size_t overflow

2016-02-15 Thread Jeff King
Performing computations on size_t variables that we feed to xmalloc and friends can be dangerous, as an integer overflow can cause us to allocate a much smaller chunk than we realized. We already have unsigned_add_overflows(), but let's add unsigned_mult_overflows() to that. Furthermore, rather

Re: [PATCH] http: add option to try authentication without username

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 4:39 PM, Junio C Hamano wrote: > "brian m. carlson" writes: >> On Mon, Feb 15, 2016 at 03:34:51PM -0500, Jeff King wrote: >>> So I think this hack should remain purely at the curl level, and never >>> touch the credential

malloc memory corruption on merge-tree with leading newline

2016-02-15 Thread Stefan Frühwirth
Hi, in one specific circumstance, git-merge-tree exits with a segfault caused by "*** Error in `git': malloc(): memory corruption (fast)": There has to be at least one commit first (as far as I can tell it doesn't matter what content). Then create a tree containing a file with a leading

[PATCH 0/18] hardening allocations against integer overflow

2016-02-15 Thread Jeff King
About 6 months or so ago, I did an audit of git's code base for uses of strcpy and sprintf that could overflow, fixing any bugs and cleaning up any suspect spots to make further audits simpler. This is a continuation of that work, for size computations which can overflow and cause us to allocate

Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value

2016-02-15 Thread Junio C Hamano
Ramsay Jones writes: >> +--show-origin:: >> +Augment the output of all queried config options with the >> +origin type (file, stdin, blob, cmdline) and the actual origin > > file, blob, cmdline (hmm, maybe cmd-line? ;-) ) If we are going to spell it out,

Re: [PATCH] http: add option to try authentication without username

2016-02-15 Thread brian m. carlson
On Mon, Feb 15, 2016 at 01:39:30PM -0800, Junio C Hamano wrote: > "brian m. carlson" writes: > > > On Mon, Feb 15, 2016 at 03:34:51PM -0500, Jeff King wrote: > > ... > >> So I think this hack should remain purely at the curl level, and never > >> touch the

Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 09:36:23PM +, Ramsay Jones wrote: > > +test_expect_success '--show-origin stdin' ' > > + cat >expect <<-\EOF && > > + stdin: user.custom=true > > So, as with the previous patch, I think this should be: > file:user.custom=true That's

Re: [PATCH] http: add option to try authentication without username

2016-02-15 Thread Junio C Hamano
"brian m. carlson" writes: > On Mon, Feb 15, 2016 at 03:34:51PM -0500, Jeff King wrote: > ... >> So I think this hack should remain purely at the curl level, and never >> touch the credential struct at all. >> >> Which is a shame, because I think Eric's suggestion

Re: [PATCH 2/4] remote: simplify remote_is_configured()

2016-02-15 Thread Junio C Hamano
Thomas Gummerer writes: > Agreed, will change. Thanks. Thanks for working on this, and thanks reviewers for helping. -- 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

Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value

2016-02-15 Thread Ramsay Jones
On 15/02/16 10:17, larsxschnei...@gmail.com wrote: > From: Lars Schneider > > If config values are queried using 'git config' (e.g. via --get, > --get-all, --get-regexp, or --list flag) then it is sometimes hard to > find the configuration file where the values were

Re: 'Failed to create .git/index.lock'

2016-02-15 Thread Andreas Krey
On Mon, 15 Feb 2016 18:06:33 +, Lars Schneider wrote: > Hi Andreas, > > I am looking into a similar issue with SourceTree on Windows right now. > However, in my case it only happens when I switch branches. Semi-bingo. That is actually a difference between the workspace this happens in and

Re: [PULL] svn pathnameencoding for git svn dcommit

2016-02-15 Thread Junio C Hamano
Eric Wong writes: > I've amended tests to both commits, but the URL encoding one > requires an HTTP server to test effectively. > > I couldn't find a test prereq for httpd, but perhaps it's good > to test by default regardless in case a future SVN changes > file://

Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type

2016-02-15 Thread Ramsay Jones
On 15/02/16 10:17, larsxschnei...@gmail.com wrote: > From: Lars Schneider > > Use the config type to print more detailed error messages that inform > the user about the origin of a config error (file, stdin, blob). > > Signed-off-by: Lars Schneider

Re: What's cooking in git.git (Feb 2016, #04; Fri, 12)

2016-02-15 Thread Junio C Hamano
Torsten Bögershausen writes: > On 15/02/16 05:50, Junio C Hamano wrote: >> Torsten Bögershausen writes: >> * tb/conversion (2016-02-10) 6 commits (merged to 'next' on 2016-02-12 at 6faf27b) >>> Could we keep it in next for a while ? >>> >>> I found

Re: git svn dcommit doesn't support --username option for file:/// urls

2016-02-15 Thread Eric Wong
Tim Ringenbach wrote: > On Mon, Feb 15, 2016 at 4:06 AM, Eric Wong wrote: > [snip] > > Totally untested, but does flipping the order of auth providers > > help at all? > > Thanks for looking into this. Unfortunately, that didn't seem to make > a

Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 03:58:12PM -0500, Eric Sunshine wrote: > > diff --git a/builtin/config.c b/builtin/config.c > > @@ -27,6 +28,7 @@ static int actions, types; > > static const char *get_color_slot, *get_colorbool_slot; > > static int end_null; > > Not related to your changes, but I just

Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 5:17 AM, wrote: > If config values are queried using 'git config' (e.g. via --get, > --get-all, --get-regexp, or --list flag) then it is sometimes hard to > find the configuration file where the values were defined. > > Teach 'git config' the

Re: [PATCH 4/4] remote: use remote_is_configured() for add and rename

2016-02-15 Thread Thomas Gummerer
On 02/15, Jeff King wrote: > On Mon, Feb 15, 2016 at 06:42:30PM +0100, Thomas Gummerer wrote: > > > Both remote add and remote rename use a slightly different hand-rolled > > check if the remote exits. The hand-rolled check may have some subtle > > cases in which it might fail to detect when a

Re: [PATCH 2/4] remote: simplify remote_is_configured()

2016-02-15 Thread Thomas Gummerer
On 02/15, Jeff King wrote: > On Mon, Feb 15, 2016 at 06:42:28PM +0100, Thomas Gummerer wrote: > > > The remote_is_configured() function allows checking whether a remote > > exists or not. The function however only works if remote_get() wasn't > > called before calling it. In addition, it only

Re: [PATCH 1/4] remote: use skip_prefix

2016-02-15 Thread Thomas Gummerer
On 02/15, Jeff King wrote: > On Mon, Feb 15, 2016 at 06:42:27PM +0100, Thomas Gummerer wrote: > > > 95b567c7 ("use skip_prefix to avoid repeating strings") transformed > > calls using starts_with() and then skipping the length of the prefix to > > skip_prefix() calls. In remote.c there are a few

Re: [PATCH] http: add option to try authentication without username

2016-02-15 Thread brian m. carlson
On Mon, Feb 15, 2016 at 03:34:51PM -0500, Jeff King wrote: > On Mon, Feb 15, 2016 at 08:29:37PM +, brian m. carlson wrote: > > That would work. I was concerned about the credential_fill call > > actually prompting the user, but it appears that it doesn't do that if > > the password already

Re: [PATCH] http: add option to try authentication without username

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 08:29:37PM +, brian m. carlson wrote: > > Rather than sprinkling curl_empty_auth special cases here and there, > > would it be possible to simply set http_auth.username and > > http_auth.password to empty strings early on if they are not already > > set and

Re: [PATCH] http: add option to try authentication without username

2016-02-15 Thread brian m. carlson
On Mon, Feb 15, 2016 at 03:19:25PM -0500, Eric Sunshine wrote: > On Mon, Feb 15, 2016 at 1:44 PM, brian m. carlson > wrote: > > Performing GSS-Negotiate authentication using Kerberos does not require > > specifying a username or password, since that information is

Re: git archive should use vendor extension in pax header

2016-02-15 Thread René Scharfe
Am 06.02.2016 um 15:57 schrieb f...@fuz.su: On Sat, Feb 06, 2016 at 02:23:11PM +0100, René Scharfe wrote: Am 28.01.2016 um 00:45 schrieb f...@fuz.su: There is git get-tar-commit-id, which prints the commit ID if it finds a comment entry which looks like a hexadecimal SHA-1 hash. It's better

Re: [PATCH] http: add option to try authentication without username

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 1:44 PM, brian m. carlson wrote: > Performing GSS-Negotiate authentication using Kerberos does not require > specifying a username or password, since that information is already > included in the ticket itself. However, libcurl refuses to

Re: git svn dcommit doesn't support --username option for file:/// urls

2016-02-15 Thread Tim Ringenbach
On Mon, Feb 15, 2016 at 4:06 AM, Eric Wong wrote: [snip] > Totally untested, but does flipping the order of auth providers > help at all? Thanks for looking into this. Unfortunately, that didn't seem to make a difference. I tried several times, and I tried both with and

[PATCH] http: add option to try authentication without username

2016-02-15 Thread brian m. carlson
Performing GSS-Negotiate authentication using Kerberos does not require specifying a username or password, since that information is already included in the ticket itself. However, libcurl refuses to perform authentication if it has not been provided with a username and password. Add an option,

Re: [PATCH 1/4] remote: use skip_prefix

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 01:35:03PM -0500, Eric Sunshine wrote: > On Mon, Feb 15, 2016 at 1:18 PM, Jeff King wrote: > > This is definitely an improvement, but I think we can actually go a step > > further here, and use parse_config_key. Like: > > > > - if (starts_with(key,

Re: [PATCH 1/4] remote: use skip_prefix

2016-02-15 Thread Eric Sunshine
On Mon, Feb 15, 2016 at 1:18 PM, Jeff King wrote: > This is definitely an improvement, but I think we can actually go a step > further here, and use parse_config_key. Like: > > - if (starts_with(key, "branch.")) { > - name = key + 7; > - subkey =

Re: [PATCH 4/4] remote: use remote_is_configured() for add and rename

2016-02-15 Thread Jeff King
On Mon, Feb 15, 2016 at 06:42:30PM +0100, Thomas Gummerer wrote: > Both remote add and remote rename use a slightly different hand-rolled > check if the remote exits. The hand-rolled check may have some subtle > cases in which it might fail to detect when a remote already exists. > One such case

  1   2   >