* Re: [PATCH 1/3] gpg: Close stderr once finished with it in verify_signed_buffer()
From: Jeff King @ 2013-01-31 5:50 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git
In-Reply-To: <1359597666-10108-2-git-send-email-sboyd@codeaurora.org>
On Wed, Jan 30, 2013 at 06:01:04PM -0800, Stephen Boyd wrote:
> Failing to close the stderr pipe in verify_signed_buffer() causes
> git to run out of file descriptors if there are many calls to
> verify_signed_buffer(). An easy way to trigger this is to run
>
> git log --show-signature --merges | grep "key"
>
> on the linux kernel git repo. Eventually it will fail with
>
> error: cannot create pipe for gpg: Too many open files
> error: could not run gpg.
>
> Close the stderr pipe so that this can't happen.
I was able to easily reproduce the bug and verify your fix here.
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 0863c61..2c0bed3 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -133,6 +133,8 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
> if (gpg_output)
> strbuf_read(gpg_output, gpg.err, 0);
> ret = finish_command(&gpg);
> + if (gpg_output)
> + close(gpg.err);
The strbuf_read above will read to EOF, so it should be equivalent (and
IMHO slightly more readable) to do:
diff --git a/gpg-interface.c b/gpg-interface.c
index 0863c61..5f142f6 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -130,8 +130,10 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
write_in_full(gpg.in, payload, payload_size);
close(gpg.in);
- if (gpg_output)
+ if (gpg_output) {
strbuf_read(gpg_output, gpg.err, 0);
+ close(gpg.err);
+ }
ret = finish_command(&gpg);
unlink_or_warn(path);
But that is a minor nit; either way, the patch looks good to me.
-Peff
^ permalink raw reply related
* How to identify the users?
From: Scott Yan @ 2013-01-31 5:52 UTC (permalink / raw)
To: git
Hello everyone:
The user info of git client (user name and email) is set by the users
themselves, so , how to avoid userA pretend to be userB?
Git server could authentication the user, but it do nothing about the
user info of commit message.
For example:
There are 20 people of my team, and everyone can push to the public
repository(git server),
If I found some backdoor code in my project, and the commit record
shows it was committed by userA, so I ask userA: why do you do this?
but he told me: no, this is not my code, I have never committed such
thing. ----and yes, everyone could change his user info to userA very
easily .
so... what should I do to avoid such situations?
Thanks!
^ permalink raw reply
* Re: How to identify the users?
From: Scott Yan @ 2013-01-31 6:07 UTC (permalink / raw)
To: Andrew Ardill; +Cc: git@vger.kernel.org
In-Reply-To: <CAH5451nd81aHtaxqpkTeCNG0xpuPd8ptdxRcOgGHaYuN3Qb7WA@mail.gmail.com>
Thanks, Andrew.
you said:
--have the server reject commits that have the 'committer' set to
someone other then the authenticated user
but I don't know how to do that?
Our central repository is hosted by apache, and there are some
username and passwords saved by apache to authentication valid user,
but as I know, there are no relation between the apache username and
the git client user ino (saved in .gitconfig), so can you describe
some detail?
Regards,
Scott Yan
On Thu, Jan 31, 2013 at 1:56 PM, Andrew Ardill <andrew.ardill@gmail.com> wrote:
>
>
>
> On 31 January 2013 16:52, Scott Yan <scottyan19@gmail.com> wrote:
>>
>> The user info of git client (user name and email) is set by the users
>> themselves, so , how to avoid userA pretend to be userB?
>>
>> Git server could authentication the user, but it do nothing about the
>> user info of commit message.
>
>
> The simplest thing is to have the server reject commits that have the
> 'committer' set to someone other then the authenticated user.
>
> Of course, there are potential workflows that this would cause problems for,
> such as if you sync directly to another user's repository and then try and
> push those to a central server.
>
> The most robust system would probably involve using signed tags to verify
> what is being pushed, however I am not aware of any set-ups that have done
> this yet.
>
> Regards,
>
> Andrew Ardill
^ permalink raw reply
* Re: How to identify the users?
From: Tomas Carnecky @ 2013-01-31 6:08 UTC (permalink / raw)
To: Scott Yan, git
In-Reply-To: <CACkbei+Jby13B7rsEb3iLQM2ZSFDgrkgvrYC5M7u4yatppvLxA@mail.gmail.com>
On Thu, 31 Jan 2013 13:52:32 +0800, Scott Yan <scottyan19@gmail.com> wrote:
> Hello everyone:
>
> The user info of git client (user name and email) is set by the users
> themselves, so , how to avoid userA pretend to be userB?
>
> Git server could authentication the user, but it do nothing about the
> user info of commit message.
>
> For example:
> There are 20 people of my team, and everyone can push to the public
> repository(git server),
> If I found some backdoor code in my project, and the commit record
> shows it was committed by userA, so I ask userA: why do you do this?
> but he told me: no, this is not my code, I have never committed such
> thing. ----and yes, everyone could change his user info to userA very
> easily .
>
> so... what should I do to avoid such situations?
gitolite keeps a log of which SSH user pushed which commits. The smart-http
backend does the same if you have reflog enabled on the server (see the
ENVIRONMENT section in man git-http-backend). So unless someone can steal
userA's credentials (http password, ssh key) you'll be able to detect who it
really was.
^ permalink raw reply
* Re: How to identify the users?
From: Sitaram Chamarty @ 2013-01-31 6:10 UTC (permalink / raw)
To: Tomas Carnecky; +Cc: Scott Yan, git
In-Reply-To: <1359612481-ner-5936@calvin>
On 01/31/2013 11:38 AM, Tomas Carnecky wrote:
> On Thu, 31 Jan 2013 13:52:32 +0800, Scott Yan <scottyan19@gmail.com> wrote:
>> Hello everyone:
>>
>> The user info of git client (user name and email) is set by the users
>> themselves, so , how to avoid userA pretend to be userB?
>>
>> Git server could authentication the user, but it do nothing about the
>> user info of commit message.
>>
>> For example:
>> There are 20 people of my team, and everyone can push to the public
>> repository(git server),
>> If I found some backdoor code in my project, and the commit record
>> shows it was committed by userA, so I ask userA: why do you do this?
>> but he told me: no, this is not my code, I have never committed such
>> thing. ----and yes, everyone could change his user info to userA very
>> easily .
>>
>> so... what should I do to avoid such situations?
>
> gitolite keeps a log of which SSH user pushed which commits. The smart-http
> backend does the same if you have reflog enabled on the server (see the
> ENVIRONMENT section in man git-http-backend). So unless someone can steal
> userA's credentials (http password, ssh key) you'll be able to detect who it
> really was.
See also my rant on this topic:
https://github.com/sitaramc/gitolite/blob/master/src/VREF/EMAIL-CHECK#L37
^ permalink raw reply
* Re: How to identify the users?
From: Andrew Ardill @ 2013-01-31 6:16 UTC (permalink / raw)
To: Scott Yan; +Cc: git@vger.kernel.org
In-Reply-To: <CACkbei+Jby13B7rsEb3iLQM2ZSFDgrkgvrYC5M7u4yatppvLxA@mail.gmail.com>
(resending previous response. Forgot to turn off HTML, and apprently
gmail doesn't wrap lines automatically anymore?)
On 31 January 2013 16:52, Scott Yan <scottyan19@gmail.com> wrote:
>
> The user info of git client (user name and email) is set by the users
> themselves, so , how to avoid userA pretend to be userB?
>
> Git server could authentication the user, but it do nothing about the
> user info of commit message.
The simplest thing is to have the server reject commits that have the
'committer' set to someone other then the authenticated user.
Of course, there are potential workflows that this would cause problems
for, such as if you sync directly to another user's repository and then try
and push those to a central server.
The most robust system would probably involve using signed tags to
verify what is being pushed, however I am not aware of any set-ups that
have done this yet.
Regards,
Andrew Ardill
^ permalink raw reply
* [PATCH 0/2] improve "git branch --contains=<commit> <pattern>"
From: Jeff King @ 2013-01-31 6:43 UTC (permalink / raw)
To: Peter Wu; +Cc: git
In-Reply-To: <679787987.uZRbjA7AMj@al>
On Wed, Jan 30, 2013 at 07:57:03PM +0100, Peter Wu wrote:
> Hi,
>
> I was trying to check whether a certain branch contained a commit and ran:
>
> git branch --contains ddc150f7a33ae0c9cb16eaac3641abc00f56316f master
>
> This resulted in:
>
> fatal: A branch named 'master' already exists.
> When "name" does not exist, this command creates a branch. I expect this
> command to search the mentioned branch, not trying to create it. The manual
> page of git-branch(1) does not mention such special behavior either.
Yeah, it's sort-of a bug. It is a syntactic ambiguity that an argument
to git-branch could be a listing pattern or a new branch name. When
using a listing pattern, you need to explicitly specify that you want
list mode with `--list`. This is documented in git-branch under --list,
but it should be more prominent, in the section that covers the various
invocation modes. The first patch below fixes that.
That being said, we could be much more helpful. It seems like --contains
should imply listing mode, since it is nonsensical in other modes. The
second patch below adjusts that, and makes the command above do what you
expect.
[1/2]: docs: clarify git-branch --list behavior
[2/2]: branch: let branch filters imply --list
-Peff
^ permalink raw reply
* [PATCH 1/2] docs: clarify git-branch --list behavior
From: Jeff King @ 2013-01-31 6:45 UTC (permalink / raw)
To: Peter Wu; +Cc: git
In-Reply-To: <20130131064357.GA24660@sigill.intra.peff.net>
It was not clear from the "description" section of
git-branch(1) that using a <pattern> meant that you _had_ to
use the --list option. Let's clarify that, and while we're
at it, reword some clunky and ambiguous sentences.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git-branch.txt | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 45a225e..01aa87f 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -22,13 +22,15 @@ DESCRIPTION
DESCRIPTION
-----------
-With no arguments, existing branches are listed and the current branch will
-be highlighted with an asterisk. Option `-r` causes the remote-tracking
-branches to be listed, and option `-a` shows both. This list mode is also
-activated by the `--list` option (see below).
-<pattern> restricts the output to matching branches, the pattern is a shell
-wildcard (i.e., matched using fnmatch(3)).
-Multiple patterns may be given; if any of them matches, the branch is shown.
+If `--list` is given, or if there are no non-option arguments, existing
+branches are listed; the current branch will be highlighted with an
+asterisk. Option `-r` causes the remote-tracking branches to be listed,
+and option `-a` shows both local and remote branches. If a `<pattern>`
+is given, it is used as a shell wildcard to restrict the output to
+matching branches. If multiple patterns are given, a branch is shown if
+any it is matched by any of the patterns. Note that when providing a
+`<pattern>`, you must use `--list`; otherwise the command is interpreted
+as branch creation.
With `--contains`, shows only the branches that contain the named commit
(in other words, the branches whose tip commits are descendants of the
--
1.8.1.2.5.g1cb3f73
^ permalink raw reply related
* [PATCH 2/2] branch: let branch filters imply --list
From: Jeff King @ 2013-01-31 6:46 UTC (permalink / raw)
To: Peter Wu; +Cc: git
In-Reply-To: <20130131064357.GA24660@sigill.intra.peff.net>
Currently, a branch filter like `--contains`, `--merged`, or
`--no-merged` is ignored when we are not in listing mode.
For example:
git branch --contains=foo bar
will create the branch "bar" from the current HEAD, ignoring
the `--contains` argument entirely. This is not very
helpful. There are two reasonable behaviors for git here:
1. Flag an error; the arguments do not make sense.
2. Implicitly go into `--list` mode
This patch chooses the latter, as it is more convenient, and
there should not be any ambiguity with attempting to create
a branch; using `--contains` and not wanting to list is
nonsensical.
That leaves the case where an explicit modification option
like `-d` is given. We already catch the case where
`--list` is given alongside `-d` and flag an error. With
this patch, we will also catch the use of `--contains` and
other filter options alongside `-d`.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git-branch.txt | 6 +++---
builtin/branch.c | 3 +++
t/t3201-branch-contains.sh | 35 +++++++++++++++++++++++++++++++++++
3 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 01aa87f..07ef5af 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -195,15 +195,15 @@ start-point is either a local or remote-tracking branch.
--contains [<commit>]::
Only list branches which contain the specified commit (HEAD
- if not specified).
+ if not specified). Implies `--list`.
--merged [<commit>]::
Only list branches whose tips are reachable from the
- specified commit (HEAD if not specified).
+ specified commit (HEAD if not specified). Implies `--list`.
--no-merged [<commit>]::
Only list branches whose tips are not reachable from the
- specified commit (HEAD if not specified).
+ specified commit (HEAD if not specified). Implies `--list`.
<branchname>::
The name of the branch to create or delete.
diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..4aa3d4e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -825,6 +825,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
list = 1;
+ if (with_commit || merge_filter != NO_FILTER)
+ list = 1;
+
if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
usage_with_options(builtin_branch_usage, options);
diff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh
index f86f4bc..141b061 100755
--- a/t/t3201-branch-contains.sh
+++ b/t/t3201-branch-contains.sh
@@ -55,6 +55,16 @@ test_expect_success 'branch --contains=side' '
'
+test_expect_success 'branch --contains with pattern implies --list' '
+
+ git branch --contains=master master >actual &&
+ {
+ echo " master"
+ } >expect &&
+ test_cmp expect actual
+
+'
+
test_expect_success 'side: branch --merged' '
git branch --merged >actual &&
@@ -66,6 +76,16 @@ test_expect_success 'side: branch --merged' '
'
+test_expect_success 'branch --merged with pattern implies --list' '
+
+ git branch --merged=side master >actual &&
+ {
+ echo " master"
+ } >expect &&
+ test_cmp expect actual
+
+'
+
test_expect_success 'side: branch --no-merged' '
git branch --no-merged >actual &&
@@ -95,4 +115,19 @@ test_expect_success 'master: branch --no-merged' '
'
+test_expect_success 'branch --no-merged with pattern implies --list' '
+
+ git branch --no-merged=master master >actual &&
+ >expect &&
+ test_cmp expect actual
+
+'
+
+test_expect_success 'implicit --list conflicts with modification options' '
+
+ test_must_fail git branch --contains=master -d &&
+ test_must_fail git branch --contains=master -m foo
+
+'
+
test_done
--
1.8.1.2.5.g1cb3f73
^ permalink raw reply related
* Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-01-31 6:49 UTC (permalink / raw)
To: Jongman Heo; +Cc: git
In-Reply-To: <28799936.346521359596121253.JavaMail.weblogic@epmltmp3>
On Thu, Jan 31, 2013 at 01:35:21AM +0000, Jongman Heo wrote:
> Looks like following commit causes a segmentation fault in my machine
> (when running git pull or git fetch);
>
> commit 8dd5afc926acb9829ebf56e9b78826a5242cd638
> Author: Junio C Hamano <gitster@pobox.com>
> Date: Mon Jan 7 12:24:55 2013 -0800
>
> string-list: allow case-insensitive string list
>
>
> In my case, list->cmp (at get_entry_index() function) has an invalid
> address, obviously not an address of string comparision function,
> instead it points to 1.
Can you show us a stack trace? The string-list functions are generic and
get called in a lot of places. It would be useful to know which list is
causing the problem.
-Peff
^ permalink raw reply
* Re: How to identify the users?
From: Scott Yan @ 2013-01-31 6:53 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: Tomas Carnecky, git@vger.kernel.org
In-Reply-To: <510A0ACF.5060501@gmail.com>
Thanks to all.
Tomas:
I can't find reflog setting of git-http-backend
doc(http://www.kernel.org/pub/software/scm/git/docs/git-http-backend.html),
I tried this setting:
git config core.logAllRefUpdates true
and after some test push, the output is as below:
>git log -g master
commit d34e61baa28eabf46ba5e9f6a2feb24cc683ed39
Reflog: master@{0} (Scott Yan <scottyan19@gmail.com>)
Reflog message: push
Author: Scott Yan <scottyan19@gmail.com>
Date: Thu Jan 31 14:19:30 2013 +0800
this log shows when pushed, but still can't tell Who, because the
author info may be fake.
I don't know if I made some mistake.
Sitaram:
It seems I must host my central repo on Gitolite first...
I don't know Gitolite much, but you are right, maybe I should use
Gitolite as my git server.
I'll find more documents about gitolite these days,
can you give me some suggestion which tutorial should I read? Thanks!
ps: my OS is windows.
Regards,
Scott Yan
On Thu, Jan 31, 2013 at 2:10 PM, Sitaram Chamarty <sitaramc@gmail.com> wrote:
> On 01/31/2013 11:38 AM, Tomas Carnecky wrote:
>> On Thu, 31 Jan 2013 13:52:32 +0800, Scott Yan <scottyan19@gmail.com> wrote:
>>> Hello everyone:
>>>
>>> The user info of git client (user name and email) is set by the users
>>> themselves, so , how to avoid userA pretend to be userB?
>>>
>>> Git server could authentication the user, but it do nothing about the
>>> user info of commit message.
>>>
>>> For example:
>>> There are 20 people of my team, and everyone can push to the public
>>> repository(git server),
>>> If I found some backdoor code in my project, and the commit record
>>> shows it was committed by userA, so I ask userA: why do you do this?
>>> but he told me: no, this is not my code, I have never committed such
>>> thing. ----and yes, everyone could change his user info to userA very
>>> easily .
>>>
>>> so... what should I do to avoid such situations?
>>
>> gitolite keeps a log of which SSH user pushed which commits. The smart-http
>> backend does the same if you have reflog enabled on the server (see the
>> ENVIRONMENT section in man git-http-backend). So unless someone can steal
>> userA's credentials (http password, ssh key) you'll be able to detect who it
>> really was.
>
> See also my rant on this topic:
>
> https://github.com/sitaramc/gitolite/blob/master/src/VREF/EMAIL-CHECK#L37
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Antoine Pelisse @ 2013-01-31 7:02 UTC (permalink / raw)
To: Jeff King; +Cc: Jongman Heo, git
In-Reply-To: <20130131064921.GB24660@sigill.intra.peff.net>
In "clean.c" we have a "string_list" created on the stack with
"STRING_LIST_INIT_NODUP" (there are probably others, I stopped at the
first occurrence).
But, "STRING_LIST_INIT_NODUP" doesn't init the "list->cmp" pointer
which can thus be random.
I don't have much time to provide a patch right now (have to go to
work), but will tonight if still needed.
Antoine,
On Thu, Jan 31, 2013 at 7:49 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Jan 31, 2013 at 01:35:21AM +0000, Jongman Heo wrote:
>
>> Looks like following commit causes a segmentation fault in my machine
>> (when running git pull or git fetch);
>>
>> commit 8dd5afc926acb9829ebf56e9b78826a5242cd638
>> Author: Junio C Hamano <gitster@pobox.com>
>> Date: Mon Jan 7 12:24:55 2013 -0800
>>
>> string-list: allow case-insensitive string list
>>
>>
>> In my case, list->cmp (at get_entry_index() function) has an invalid
>> address, obviously not an address of string comparision function,
>> instead it points to 1.
>
> Can you show us a stack trace? The string-list functions are generic and
> get called in a lot of places. It would be useful to know which list is
> causing the problem.
>
> -Peff
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Re: Segmentation fault with latest git (070c57df)
From: Jongman Heo @ 2013-01-31 7:27 UTC (permalink / raw)
To: git, Antoine Pelisse, Jeff King
On Thu, Jan 31, 2013 at 7:49 AM, Jeff King wrote:
> On Thu, Jan 31, 2013 at 01:35:21AM +0000, Jongman Heo wrote:
>
>> Looks like following commit causes a segmentation fault in my machine
>> (when running git pull or git fetch);
>>
>> commit 8dd5afc926acb9829ebf56e9b78826a5242cd638
>> Author: Junio C Hamano
>> Date: Mon Jan 7 12:24:55 2013 -0800
>>
>> string-list: allow case-insensitive string list
>>
>>
>> In my case, list->cmp (at get_entry_index() function) has an invalid
>> address, obviously not an address of string comparision function,
>> instead it points to 1.
>
> Can you show us a stack trace? The string-list functions are generic and
> get called in a lot of places. It would be useful to know which list is
> causing the problem.
>
> -Peff
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
FYI, gdb backtrace and valgrind output attached below, Thanks.
(gdb) run fetch
Starting program: /home/hjongman/repos/git/git fetch
warning: .dynamic section for "/lib/libc.so.6" is not at the expected address
warning: difference appears to be caused by prelink, adjusting expectations
[Thread debugging using libthread_db enabled]
Program received signal SIGSEGV, Segmentation fault.
0x00000001 in ?? ()
(gdb) bt
#0 0x00000001 in ?? ()
#1 0x0812b457 in get_entry_index (list=0xbfffe7c0, string=0x821ec3c "refs/remotes/origin/HEAD", exact_match=0xbfffe568)
at string-list.c:14
#2 0x0812bd60 in add_entry (list=0xbfffe7c0, insert_at=-1, string=0x821ec3c "refs/remotes/origin/HEAD") at string-list.c:33
#3 string_list_insert_at_index (list=0xbfffe7c0, insert_at=-1, string=0x821ec3c "refs/remotes/origin/HEAD") at string-list.c:63
#4 0x0812bda0 in string_list_insert (list=0xbfffe7c0, string=0x821ec3c "refs/remotes/origin/HEAD") at string-list.c:57
#5 0x08071838 in add_existing (refname=0x821ec3c "refs/remotes/origin/HEAD",
sha1=0x821ec14 "\a\fW\337B\352N\255\314C\320Em\021E`\022C&", <incomplete sequence \303>, flag=1, cbdata=0xbfffe7c0)
at builtin/fetch.c:570
#6 0x0810af97 in do_one_ref (base=<value optimized out>, fn=0x8071820 <add_existing>, trim=0, flags=<value optimized out>,
cb_data=0xbfffe7c0, entry=0x821ec10) at refs.c:525
#7 0x0810bd9f in do_for_each_ref_in_dirs (dir1=0x8215d54, dir2=0x821ea44, base=0x814f9ff "", fn=0x8071820 <add_existing>, trim=0,
flags=0, cb_data=0xbfffe7c0) at refs.c:627
#8 0x0810bc8e in do_for_each_ref_in_dirs (dir1=0x8215cac, dir2=0x8226954, base=0x814f9ff "", fn=0x8071820 <add_existing>, trim=0,
flags=0, cb_data=0xbfffe7c0) at refs.c:597
#9 0x0810bc8e in do_for_each_ref_in_dirs (dir1=0x8215c0c, dir2=0x8215a54, base=0x814f9ff "", fn=0x8071820 <add_existing>, trim=0,
flags=0, cb_data=0xbfffe7c0) at refs.c:597
#10 0x0810bc8e in do_for_each_ref_in_dirs (dir1=0x8215a1c, dir2=0x821862c, base=0x814f9ff "", fn=0x8071820 <add_existing>, trim=0,
flags=0, cb_data=0xbfffe7c0) at refs.c:597
#11 0x0810c304 in do_for_each_ref (submodule=<value optimized out>, base=0x814f9ff "", fn=0x8071820 <add_existing>, trim=0, flags=0,
cb_data=0xbfffe7c0) at refs.c:1295
#12 0x0810c63b in for_each_ref (fn=0x8071820 <add_existing>, cb_data=0xbfffe7c0) at refs.c:1343
#13 0x0807390a in do_fetch (remote=<value optimized out>, argc=0, argv=0xbfffe9f8) at builtin/fetch.c:699
#14 fetch_one (remote=<value optimized out>, argc=0, argv=0xbfffe9f8) at builtin/fetch.c:949
#15 0x08074251 in cmd_fetch (argc=1, argv=0xbfffe9f8, prefix=0x0) at builtin/fetch.c:992
#16 0x0804b60b in run_builtin (argc=1, argv=0xbfffe9f8) at git.c:281
#17 handle_internal_command (argc=1, argv=0xbfffe9f8) at git.c:443
#18 0x0804ba51 in run_argv (argc=1, argv=0xbfffe9f8) at git.c:489
#19 main (argc=1, argv=0xbfffe9f8) at git.c:564
$ valgrind git fetch
==2195== Memcheck, a memory error detector
==2195== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==2195== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==2195== Command: git fetch
==2195==
==2195== Conditional jump or move depends on uninitialised value(s)
==2195== at 0x812B41F: get_entry_index (string-list.c:10)
==2195== by 0x812BD5F: string_list_insert_at_index (string-list.c:33)
==2195== by 0x812BD9F: string_list_insert (string-list.c:57)
==2195== by 0x8071837: add_existing (fetch.c:570)
==2195== by 0x810AF96: do_one_ref (refs.c:525)
==2195== by 0x810BB20: do_for_each_ref_in_dir (refs.c:551)
==2195== by 0x810BD34: do_for_each_ref_in_dirs (refs.c:623)
==2195== by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
==2195== by 0x810C303: do_for_each_ref (refs.c:1295)
==2195== by 0x810C63A: for_each_ref (refs.c:1343)
==2195== by 0x8073909: fetch_one (fetch.c:699)
==2195== by 0x8074250: cmd_fetch (fetch.c:992)
==2195==
==2195== Use of uninitialised value of size 4
==2195== at 0x812B454: get_entry_index (string-list.c:14)
==2195== by 0x812BD5F: string_list_insert_at_index (string-list.c:33)
==2195== by 0x812BD9F: string_list_insert (string-list.c:57)
==2195== by 0x8071837: add_existing (fetch.c:570)
==2195== by 0x810AF96: do_one_ref (refs.c:525)
==2195== by 0x810BD9E: do_for_each_ref_in_dirs (refs.c:627)
==2195== by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
==2195== by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
==2195== by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
==2195== by 0x810C303: do_for_each_ref (refs.c:1295)
==2195== by 0x810C63A: for_each_ref (refs.c:1343)
==2195== by 0x8073909: fetch_one (fetch.c:699)
==2195==
==2195==
==2195== Process terminating with default action of signal 11 (SIGSEGV)
==2195== Access not within mapped region at address 0x0
==2195== at 0x1: ???
==2195== by 0x812BD5F: string_list_insert_at_index (string-list.c:33)
==2195== by 0x812BD9F: string_list_insert (string-list.c:57)
==2195== by 0x8071837: add_existing (fetch.c:570)
==2195== by 0x810AF96: do_one_ref (refs.c:525)
==2195== by 0x810BD9E: do_for_each_ref_in_dirs (refs.c:627)
==2195== by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
==2195== by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
==2195== by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
==2195== by 0x810C303: do_for_each_ref (refs.c:1295)
==2195== by 0x810C63A: for_each_ref (refs.c:1343)
==2195== by 0x8073909: fetch_one (fetch.c:699)
==2195== If you believe this happened as a result of a stack
==2195== overflow in your program's main thread (unlikely but
==2195== possible), you can try to increase the size of the
==2195== main thread stack using the --main-stacksize= flag.
==2195== The main thread stack size used in this run was 10485760.
==2195==
==2195== HEAP SUMMARY:
==2195== in use at exit: 325,604 bytes in 3,579 blocks
==2195== total heap usage: 3,687 allocs, 108 frees, 518,058 bytes allocated
==2195==
==2195== LEAK SUMMARY:
==2195== definitely lost: 72 bytes in 2 blocks
==2195== indirectly lost: 0 bytes in 0 blocks
==2195== possibly lost: 31,347 bytes in 460 blocks
==2195== still reachable: 294,185 bytes in 3,117 blocks
==2195== suppressed: 0 bytes in 0 blocks
==2195== Rerun with --leak-check=full to see details of leaked memory
==2195==
==2195== For counts of detected and suppressed errors, rerun with: -v
==2195== Use --track-origins=yes to see where uninitialised values come from
==2195== ERROR SUMMARY: 3 errors from 2 contexts (suppressed: 17 from 8)
Segmentation fault
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-01-31 7:35 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: Jongman Heo, git
In-Reply-To: <CALWbr2y8veWYRwEGrbGq-PhxZjXbBF5HMgQXrRr=1JzGQ9Y-tQ@mail.gmail.com>
On Thu, Jan 31, 2013 at 08:02:06AM +0100, Antoine Pelisse wrote:
> In "clean.c" we have a "string_list" created on the stack with
> "STRING_LIST_INIT_NODUP" (there are probably others, I stopped at the
> first occurrence).
> But, "STRING_LIST_INIT_NODUP" doesn't init the "list->cmp" pointer
> which can thus be random.
I don't think that is the problem. Extra struct members that are not
mentioned by an initializer will get set to 0 or NULL.
A code path that tried to initialize each member individually would run
into problems, but I could not find any such code path (all of them
either use an initializer, are static, or memset the struct to
all-zeroes).
-Peff
^ permalink raw reply
* Re: Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-01-31 7:55 UTC (permalink / raw)
To: Jongman Heo; +Cc: git, Antoine Pelisse
In-Reply-To: <15825158.309231359617223702.JavaMail.weblogic@epml26>
On Thu, Jan 31, 2013 at 07:27:04AM +0000, Jongman Heo wrote:
> FYI, gdb backtrace and valgrind output attached below, Thanks.
Thanks, that's helpful.
> #4 0x0812bda0 in string_list_insert (list=0xbfffe7c0, string=0x821ec3c "refs/remotes/origin/HEAD") at string-list.c:57
> #5 0x08071838 in add_existing (refname=0x821ec3c "refs/remotes/origin/HEAD",
> sha1=0x821ec14 "\a\fW\337B\352N\255\314C\320Em\021E`\022C&", <incomplete sequence \303>, flag=1, cbdata=0xbfffe7c0)
> at builtin/fetch.c:570
So we are inserting the string from add_existing, which gets the list
from a callback parameter. Which comes from...
> #13 0x0807390a in do_fetch (remote=<value optimized out>, argc=0, argv=0xbfffe9f8) at builtin/fetch.c:699
...here, which does this:
struct string_list existing_refs = STRING_LIST_INIT_NODUP;
[...]
for_each_ref(add_existing, &existing_refs);
And yet we get:
> ==2195== Conditional jump or move depends on uninitialised value(s)
> ==2195== at 0x812B41F: get_entry_index (string-list.c:10)
> ==2195== by 0x812BD5F: string_list_insert_at_index (string-list.c:33)
> ==2195== by 0x812BD9F: string_list_insert (string-list.c:57)
> ==2195== by 0x8071837: add_existing (fetch.c:570)
> ==2195== by 0x810AF96: do_one_ref (refs.c:525)
> ==2195== by 0x810BB20: do_for_each_ref_in_dir (refs.c:551)
> ==2195== by 0x810BD34: do_for_each_ref_in_dirs (refs.c:623)
> ==2195== by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
> ==2195== by 0x810C303: do_for_each_ref (refs.c:1295)
> ==2195== by 0x810C63A: for_each_ref (refs.c:1343)
> ==2195== by 0x8073909: fetch_one (fetch.c:699)
> ==2195== by 0x8074250: cmd_fetch (fetch.c:992)
which seems odd. cmp should be initialized to NULL, and then we never
touch it (and even if we did, it wouldn't be unitialized, but rather
have the value we put in it).
It's almost like the compiler is getting the initializer wrong. It's a
long shot, but I wonder if the presence of the bitfield could be
triggering a compiler bug (or there is a subtle C rule about bitfield
initializations that I do not know). Just for the sake of my sanity,
what does the following program output for you?
-- >8 --
#include <stdio.h>
#include <stdlib.h>
typedef int (*compare_fn)(const char *, const char *);
struct foo {
char **items;
unsigned int nr, alloc;
unsigned int bitfield:1;
compare_fn cmp;
};
int main(void)
{
struct foo f = { NULL, 0, 0, 0 };
printf("cmp is %lu\n", (unsigned long)f.cmp);
return 0;
}
^ permalink raw reply
* Re: Re: Re: Segmentation fault with latest git (070c57df)
From: 허종만 @ 2013-01-31 8:07 UTC (permalink / raw)
To: Jeff King; +Cc: git, Antoine Pelisse
> It's almost like the compiler is getting the initializer wrong. It's a
> long shot, but I wonder if the presence of the bitfield could be
> triggering a compiler bug (or there is a subtle C rule about bitfield
> initializations that I do not know). Just for the sake of my sanity,
> what does the following program output for you?
Hi,
just "cmp is 0" is printed.
$ gcc --version
gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-48)
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Best regards,
Jongman Heo.
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Thomas Rast @ 2013-01-31 8:42 UTC (permalink / raw)
To: Jeff King; +Cc: Jongman Heo, git, Antoine Pelisse
In-Reply-To: <20130131075511.GB5342@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Jan 31, 2013 at 07:27:04AM +0000, Jongman Heo wrote:
>
>> FYI, gdb backtrace and valgrind output attached below, Thanks.
>
> Thanks, that's helpful.
>
>> #4 0x0812bda0 in string_list_insert (list=0xbfffe7c0,
>> string=0x821ec3c "refs/remotes/origin/HEAD") at string-list.c:57
>> #5 0x08071838 in add_existing (refname=0x821ec3c "refs/remotes/origin/HEAD",
>> sha1=0x821ec14 "\a\fW\337B\352N\255\314C\320Em\021E`\022C&",
>> <incomplete sequence \303>, flag=1, cbdata=0xbfffe7c0)
>> at builtin/fetch.c:570
>
> So we are inserting the string from add_existing, which gets the list
> from a callback parameter. Which comes from...
>
>> #13 0x0807390a in do_fetch (remote=<value optimized out>, argc=0,
>> argv=0xbfffe9f8) at builtin/fetch.c:699
>
> ...here, which does this:
>
> struct string_list existing_refs = STRING_LIST_INIT_NODUP;
> [...]
> for_each_ref(add_existing, &existing_refs);
>
> And yet we get:
>
>> ==2195== Conditional jump or move depends on uninitialised value(s)
>> ==2195== at 0x812B41F: get_entry_index (string-list.c:10)
>> ==2195== by 0x812BD5F: string_list_insert_at_index (string-list.c:33)
>> ==2195== by 0x812BD9F: string_list_insert (string-list.c:57)
>> ==2195== by 0x8071837: add_existing (fetch.c:570)
>> ==2195== by 0x810AF96: do_one_ref (refs.c:525)
>> ==2195== by 0x810BB20: do_for_each_ref_in_dir (refs.c:551)
>> ==2195== by 0x810BD34: do_for_each_ref_in_dirs (refs.c:623)
>> ==2195== by 0x810BC8D: do_for_each_ref_in_dirs (refs.c:597)
>> ==2195== by 0x810C303: do_for_each_ref (refs.c:1295)
>> ==2195== by 0x810C63A: for_each_ref (refs.c:1343)
>> ==2195== by 0x8073909: fetch_one (fetch.c:699)
>> ==2195== by 0x8074250: cmd_fetch (fetch.c:992)
>
> which seems odd. cmp should be initialized to NULL, and then we never
> touch it (and even if we did, it wouldn't be unitialized, but rather
> have the value we put in it).
>
> It's almost like the compiler is getting the initializer wrong. It's a
> long shot, but I wonder if the presence of the bitfield could be
> triggering a compiler bug (or there is a subtle C rule about bitfield
> initializations that I do not know). Just for the sake of my sanity,
> what does the following program output for you?
>
> -- >8 --
> #include <stdio.h>
> #include <stdlib.h>
>
> typedef int (*compare_fn)(const char *, const char *);
>
> struct foo {
> char **items;
> unsigned int nr, alloc;
> unsigned int bitfield:1;
> compare_fn cmp;
> };
>
> int main(void)
> {
> struct foo f = { NULL, 0, 0, 0 };
> printf("cmp is %lu\n", (unsigned long)f.cmp);
> return 0;
> }
I doubt that would help because that stack region would be 0 anyway due
to kernel initialization of new pages. You'd have to somehow trample
over it first, like below.
Or perhaps something in the build process went wrong, and fetch.c didn't
get the memo about the new field in the struct. Depending on stack
layout, the next variable might be the 'int i' right before the
'string_list list' in the code, which could explain the value of 1.
---- 8< ----
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef int (*compare_fn)(const char *, const char *);
struct foo {
char **items;
unsigned int nr, alloc;
unsigned int bitfield:1;
compare_fn cmp;
};
void scramble()
{
char foo[256];
memset(foo, 0x42, 256);
}
void init()
{
struct foo f = { NULL, 0, 0, 0 };
printf("cmp is %lu\n", (unsigned long)f.cmp);
}
int main(void)
{
scramble();
init();
return 0;
}
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-01-31 8:54 UTC (permalink / raw)
To: Thomas Rast; +Cc: Jongman Heo, git, Antoine Pelisse
In-Reply-To: <87wqutvj4w.fsf@pctrast.inf.ethz.ch>
On Thu, Jan 31, 2013 at 09:42:07AM +0100, Thomas Rast wrote:
> > int main(void)
> > {
> > struct foo f = { NULL, 0, 0, 0 };
> > printf("cmp is %lu\n", (unsigned long)f.cmp);
> > return 0;
> > }
>
> I doubt that would help because that stack region would be 0 anyway due
> to kernel initialization of new pages. You'd have to somehow trample
> over it first, like below.
Good point. Unfortunately, I can't get either yours or mine to fail,
neither with a recent version of gcc nor with gcc-4.1. But I can't
convince git to fail, either. The only gcc-4.1 I have is Debian's
4.1.3 release, which is not quite what the OP has.
> Or perhaps something in the build process went wrong, and fetch.c didn't
> get the memo about the new field in the struct. Depending on stack
> layout, the next variable might be the 'int i' right before the
> 'string_list list' in the code, which could explain the value of 1.
Yeah, that would make sense to me with respect to the behavior we are
seeing, but that part of the Makefile should be pretty simple and
bug-free, I'd think (and from the original report, it seems like he was
able to reproduce it well enough to bisect). Still, trying a "make clean
&& make" might be worth it just to rule that out.
Puzzled...
-Peff
^ permalink raw reply
* Re: How to identify the users?
From: Sitaram Chamarty @ 2013-01-31 10:12 UTC (permalink / raw)
To: Scott Yan; +Cc: Tomas Carnecky, git@vger.kernel.org
In-Reply-To: <CACkbei+_dJowH-odL+UCS3hQwOwFZ7B5_6sxw=ZZg1V4=upSKg@mail.gmail.com>
On 01/31/2013 12:23 PM, Scott Yan wrote:
> Sitaram:
>
> It seems I must host my central repo on Gitolite first...
There is no "must" but yes it is a decent solution and can, in
principle, do the kind of checking you want if you set it up to do that.
Please note that I don't use that mode and, as my rant would have
indicated, I don't think it's a smart thing to do.
> I don't know Gitolite much, but you are right, maybe I should use
> Gitolite as my git server.
> I'll find more documents about gitolite these days,
> can you give me some suggestion which tutorial should I read? Thanks!
> ps: my OS is windows.
Try
http://therightstuff.de/CommentView,guid,b969ea4d-8d2c-42af-9806-de3631f4df68.aspx
I normally don't mention blog posts (favouring instead the official
documentation) but Windows is an exception. Hence the link.
Good luck.
^ permalink raw reply
* Re: [PATCH 4/6] introduce a commit metapack
From: Duy Nguyen @ 2013-01-31 11:06 UTC (permalink / raw)
To: Jeff King; +Cc: git, Shawn O. Pearce
In-Reply-To: <CACsJy8Bqg6knVtddwaGSqtiXzVDgbO1JjbFOPMbF_RqrxM2rFQ@mail.gmail.com>
On Wed, Jan 30, 2013 at 09:16:29PM +0700, Duy Nguyen wrote:
> Perhaps we could store abbrev sha-1 instead of full sha-1. Nice
> space/time trade-off.
Following the on-disk format experiment yesterday, I changed the
format to:
- a list a _short_ SHA-1 of cached commits
- a list of cache entries, each (5 uint32_t) consists of:
- uint32_t for the index in .idx sha-1 table to get full SHA-1 of
the commit
- uint32_t for timestamp
- uint32_t for tree, 1st and 2nd parents for the index in .idx
table
The length of SHA-1 is chosen to be able to unambiguously identify any
cached commits. Full SHA-1 check is done after to catch false
positives. For linux-2.6, SHA-1 length is 6 bytes, git and many
moderate-sized projects are 4 bytes. So it's 26 bytes per commit for
linux-2.6.git, or 8MB .commits file. Not as good as revindex approach
(5.5MB) but way better than the current one (27MB). And still good
enough for caching 2 parents unconditionally.
Performance seems to improve a tiny bit, probably because of more
compact search space: 0.6s with my patch vs 0.7s without (vs 3.4s
without cache).
-- 8< --
diff --git a/cache.h b/cache.h
index 1f96f65..8048d5b 100644
--- a/cache.h
+++ b/cache.h
@@ -1069,6 +1069,7 @@ extern struct packed_git *add_packed_git(const char *, int, int);
extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t);
extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t);
extern off_t find_pack_entry_one(const unsigned char *, struct packed_git *);
+extern int find_pack_entry_pos(const unsigned char *, struct packed_git *);
extern int is_pack_valid(struct packed_git *);
extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsigned long *);
extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
diff --git a/commit-metapack.c b/commit-metapack.c
index 2c19f48..c984b8e 100644
--- a/commit-metapack.c
+++ b/commit-metapack.c
@@ -4,11 +4,21 @@
#include "commit.h"
#include "sha1-lookup.h"
+struct commit_entry {
+ uint32_t commit; /* nth_packed_object_sha1 to get own SHA-1 */
+ uint32_t timestamp;
+ uint32_t tree; /* nth_packed_object_sha1 to get tree SHA-1 */
+ uint32_t parent1; /* nth_packed_object_sha1 to get 1st parent SHA-1 */
+ uint32_t parent2; /* nth_packed_object_sha1 to get 2nd parent SHA-1 */
+};
+
struct commit_metapack {
struct metapack mp;
uint32_t nr;
+ uint32_t abbrev_len;
+ struct packed_git *pack;
unsigned char *index;
- unsigned char *data;
+ struct commit_entry *data;
struct commit_metapack *next;
};
static struct commit_metapack *commit_metapacks;
@@ -41,20 +51,23 @@ static struct commit_metapack *alloc_commit_metapack(struct packed_git *pack)
}
memcpy(&it->nr, it->mp.data, 4);
it->nr = ntohl(it->nr);
+ memcpy(&it->abbrev_len, it->mp.data + 4, 4);
+ it->abbrev_len = ntohl(it->abbrev_len);
+ it->pack = pack;
/*
- * We need 84 bytes for each entry: sha1(20), date(4), tree(20),
- * parents(40).
+ * We need 20+abbrev_len bytes for each entry: abbrev sha-1,
+ * date(4), tree index(4), parent indexes(8).
*/
- if (it->mp.len < (84 * it->nr + 4)) {
+ if (it->mp.len < ((sizeof(*it->data) + it->abbrev_len) * it->nr + 8)) {
warning("commit metapack for '%s' is truncated", pack->pack_name);
metapack_close(&it->mp);
free(it);
return NULL;
}
- it->index = it->mp.data + 4;
- it->data = it->index + 20 * it->nr;
+ it->index = it->mp.data + 8;
+ it->data = (struct commit_entry*)(it->index + it->abbrev_len * it->nr);
return it;
}
@@ -83,29 +96,51 @@ static void prepare_commit_metapacks(void)
int commit_metapack(unsigned char *sha1,
uint32_t *timestamp,
- unsigned char **tree,
- unsigned char **parent1,
- unsigned char **parent2)
+ const unsigned char **tree,
+ const unsigned char **parent1,
+ const unsigned char **parent2)
{
struct commit_metapack *p;
prepare_commit_metapacks();
for (p = commit_metapacks; p; p = p->next) {
- unsigned char *data;
- int pos = sha1_entry_pos(p->index, 20, 0, 0, p->nr, p->nr, sha1);
+ struct commit_entry *data;
+ uint32_t p1, p2;
+ unsigned lo, hi, mi;
+ int pos;
+
+ /* sha1_entry_pos does not work with abbreviated sha-1 */
+ lo = 0;
+ hi = p->nr;
+ pos = -1;
+ do {
+ unsigned mi = (lo + hi) / 2;
+ int cmp = memcmp(p->index + mi * p->abbrev_len, sha1, p->abbrev_len);
+
+ if (!cmp) {
+ pos = mi;
+ break;
+ }
+ if (cmp > 0)
+ hi = mi;
+ else
+ lo = mi+1;
+ } while (lo < hi);
if (pos < 0)
continue;
- /* timestamp(4) + tree(20) + parents(40) */
- data = p->data + 64 * pos;
- *timestamp = *(uint32_t *)data;
- *timestamp = ntohl(*timestamp);
- data += 4;
- *tree = data;
- data += 20;
- *parent1 = data;
- data += 20;
- *parent2 = data;
+ data = p->data + pos;
+
+ /* full sha-1 check again */
+ if (hashcmp(nth_packed_object_sha1(p->pack, ntohl(data->commit)), sha1))
+ continue;
+
+ *timestamp = ntohl(data->timestamp);
+ *tree = nth_packed_object_sha1(p->pack, ntohl(data->tree));
+ p1 = ntohl(data->parent1);
+ *parent1 = nth_packed_object_sha1(p->pack, p1);
+ p2 = ntohl(data->parent2);
+ *parent2 = p1 == p2 ? null_sha1 : nth_packed_object_sha1(p->pack, p2);
return 0;
}
@@ -113,13 +148,20 @@ int commit_metapack(unsigned char *sha1,
return -1;
}
+struct write_cb {
+ struct commit_list **tail;
+ int abbrev_len;
+ const unsigned char *last_sha1;
+};
+
static void get_commits(struct metapack_writer *mw,
const unsigned char *sha1,
void *data)
{
- struct commit_list ***tail = data;
+ struct write_cb *write_cb = (struct write_cb *)data;
enum object_type type = sha1_object_info(sha1, NULL);
struct commit *c;
+ int p1, p2;
if (type != OBJ_COMMIT)
return;
@@ -128,47 +170,95 @@ static void get_commits(struct metapack_writer *mw,
if (!c || parse_commit(c))
die("unable to read commit %s", sha1_to_hex(sha1));
+ if (c->buffer) {
+ free(c->buffer);
+ c->buffer = NULL;
+ }
+
/*
* Our fixed-size parent list cannot represent root commits, nor
* octopus merges. Just skip those commits, as we can fallback
* in those rare cases to reading the actual commit object.
*/
if (!c->parents ||
- (c->parents && c->parents->next && c->parents->next->next))
+ (c->parents && c->parents->next && c->parents->next->next) ||
+ /* edge commits are out too */
+ find_pack_entry_pos(c->tree->object.sha1, mw->pack) == -1 ||
+ (p1 = find_pack_entry_pos(c->parents->item->object.sha1, mw->pack)) == -1 ||
+ (c->parents->next &&
+ (p2 = find_pack_entry_pos(c->parents->next->item->object.sha1, mw->pack)) == -1) ||
+ /*
+ * we set the 2nd parent the same as 1st parent as an
+ * indication that 2nd parent does not exist. Normal
+ * commits should never have two same parents, but just in
+ * case..
+ */
+ p1 == p2)
return;
- *tail = &commit_list_insert(c, *tail)->next;
+ /*
+ * Make sure we store the abbr sha-1 long enough to
+ * unambiguously identify any cached commits in the pack.
+ */
+ while (write_cb->abbrev_len < 20 &&
+ write_cb->last_sha1 &&
+ !memcmp(write_cb->last_sha1, sha1, write_cb->abbrev_len))
+ write_cb->abbrev_len++;
+ /*
+ * A bit sensitive to metapack_writer_foreach. "sha1" must not
+ * be changed even after this function exits.
+ */
+ write_cb->last_sha1 = sha1;
+
+ write_cb->tail = &commit_list_insert(c, write_cb->tail)->next;
}
void commit_metapack_write(const char *idx)
{
struct metapack_writer mw;
struct commit_list *commits = NULL, *p;
- struct commit_list **tail = &commits;
+ struct write_cb write_cb;
uint32_t nr = 0;
metapack_writer_init(&mw, idx, "commits", 1);
+ write_cb.tail = &commits;
+ write_cb.abbrev_len = 1;
+ write_cb.last_sha1 = NULL;
+
/* Figure out how many eligible commits we've got in this pack. */
- metapack_writer_foreach(&mw, get_commits, &tail);
+ metapack_writer_foreach(&mw, get_commits, &write_cb);
for (p = commits; p; p = p->next)
nr++;
+
metapack_writer_add_uint32(&mw, nr);
+ metapack_writer_add_uint32(&mw, write_cb.abbrev_len);
/* Then write an index of commit sha1s */
for (p = commits; p; p = p->next)
- metapack_writer_add(&mw, p->item->object.sha1, 20);
+ metapack_writer_add(&mw, p->item->object.sha1, write_cb.abbrev_len);
/* Followed by the actual date/tree/parents data */
for (p = commits; p; p = p->next) {
struct commit *c = p->item;
+ int pos;
+
+ pos = find_pack_entry_pos(c->object.sha1, mw.pack);
+ metapack_writer_add_uint32(&mw, pos);
+
metapack_writer_add_uint32(&mw, c->date);
- metapack_writer_add(&mw, c->tree->object.sha1, 20);
- metapack_writer_add(&mw, c->parents->item->object.sha1, 20);
- metapack_writer_add(&mw,
- c->parents->next ?
- c->parents->next->item->object.sha1 :
- null_sha1, 20);
+
+ pos = find_pack_entry_pos(c->tree->object.sha1, mw.pack);
+ metapack_writer_add_uint32(&mw, pos);
+
+ pos = find_pack_entry_pos(c->parents->item->object.sha1, mw.pack);
+ metapack_writer_add_uint32(&mw, pos);
+
+ if (c->parents->next) {
+ struct object *o = &c->parents->next->item->object;
+ pos = find_pack_entry_pos(o->sha1, mw.pack);
+ }
+ metapack_writer_add_uint32(&mw, pos);
}
metapack_writer_finish(&mw);
diff --git a/commit-metapack.h b/commit-metapack.h
index 4684573..caf85be 100644
--- a/commit-metapack.h
+++ b/commit-metapack.h
@@ -3,9 +3,9 @@
int commit_metapack(unsigned char *sha1,
uint32_t *timestamp,
- unsigned char **tree,
- unsigned char **parent1,
- unsigned char **parent2);
+ const unsigned char **tree,
+ const unsigned char **parent1,
+ const unsigned char **parent2);
void commit_metapack_write(const char *idx_file);
diff --git a/sha1_file.c b/sha1_file.c
index 40b2329..1acab8c 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1978,8 +1978,8 @@ off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n)
}
}
-off_t find_pack_entry_one(const unsigned char *sha1,
- struct packed_git *p)
+int find_pack_entry_pos(const unsigned char *sha1,
+ struct packed_git *p)
{
const uint32_t *level1_ofs = p->index_data;
const unsigned char *index = p->index_data;
@@ -1992,7 +1992,7 @@ off_t find_pack_entry_one(const unsigned char *sha1,
if (!index) {
if (open_pack_index(p))
- return 0;
+ return -1;
level1_ofs = p->index_data;
index = p->index_data;
}
@@ -2019,9 +2019,7 @@ off_t find_pack_entry_one(const unsigned char *sha1,
if (use_lookup) {
int pos = sha1_entry_pos(index, stride, 0,
lo, hi, p->num_objects, sha1);
- if (pos < 0)
- return 0;
- return nth_packed_object_offset(p, pos);
+ return pos;
}
do {
@@ -2032,15 +2030,24 @@ off_t find_pack_entry_one(const unsigned char *sha1,
printf("lo %u hi %u rg %u mi %u\n",
lo, hi, hi - lo, mi);
if (!cmp)
- return nth_packed_object_offset(p, mi);
+ return mi;
if (cmp > 0)
hi = mi;
else
lo = mi+1;
} while (lo < hi);
- return 0;
+ return -1;
}
+off_t find_pack_entry_one(const unsigned char *sha1,
+ struct packed_git *p)
+{
+ int pos = find_pack_entry_pos(sha1, p);
+ if (pos < 0)
+ return 0;
+ else
+ return nth_packed_object_offset(p, pos);
+}
int is_pack_valid(struct packed_git *p)
{
/* An already open pack is known to be valid. */
-- 8< --
^ permalink raw reply related
* [feature request] git-daemon http connection filtering of client types
From: @ 2013-01-31 12:46 UTC (permalink / raw)
To: git
Hey folks,
When I checked for false positives in my spam this morning, I spotted
an interesting malformed img link at the top of a spam message.
{snip}
> <http://git.{snip}.n2.nabble.com/file/{snip}/t3.jpg>
>
> Employ a medal tiffany bracelet <{snip}> a is
{snip}
So, apparently git-daemon's http features are being used by spammers.
In most cases, spam filters will correctly identify this junk.
I wonder if there is a better way... In my mental sandbox, git-daemon
http could have a set of deny/allow rules for incoming connection
client types.
e.g.:
git: allow
git-http: allow
thunderbird: deny
outlook express: replace linked file with rickroll.jpg
and so on.. An out-of-the-box install probably should default to
allow all to keep backward compatibility.
While I'd love a chance to hack something out, I sadly doubt I'll ever
have the time for it. Perhaps there is a student hacker looking for a
project.
Cheers!
-phil
p.s. appologies to anyone who now has Astley's song stuck in their
head. This was not intentional.
^ permalink raw reply
* [PATCH] push: fix segfault when HEAD points nowhere
From: Fraser Tweedale @ 2013-01-31 12:22 UTC (permalink / raw)
To: git; +Cc: Fraser Tweedale
When reporting the outcome of a push, a segfault occurs if HEAD does
not point somewhere. Check that HEAD points somewhere before trying
to strcmp it.
Signed-off-by: Fraser Tweedale <frase@frase.id.au>
---
transport.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/transport.c b/transport.c
index 9932f40..b9306ef 100644
--- a/transport.c
+++ b/transport.c
@@ -741,7 +741,7 @@ void transport_print_push_status(const char *dest, struct ref *refs,
n += print_one_push_status(ref, dest, n, porcelain);
if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD &&
*nonfastforward != NON_FF_HEAD) {
- if (!strcmp(head, ref->name))
+ if (head != NULL && !strcmp(head, ref->name))
*nonfastforward = NON_FF_HEAD;
else
*nonfastforward = NON_FF_OTHER;
--
1.8.1.1
^ permalink raw reply related
* Re: [feature request] git-daemon http connection filtering of client types
From: Erik Faye-Lund @ 2013-01-31 13:22 UTC (permalink / raw)
To: porpen+git; +Cc: git
In-Reply-To: <CAANzHtT83JXhQ8XRifdK5ah7NrnD6hvrCjBO4PnPx=qC=DTT9A@mail.gmail.com>
On Thu, Jan 31, 2013 at 1:46 PM, <porpen+git@gmail.com> wrote:
> Hey folks,
>
> When I checked for false positives in my spam this morning, I spotted
> an interesting malformed img link at the top of a spam message.
>
> {snip}
>> <http://git.{snip}.n2.nabble.com/file/{snip}/t3.jpg>
>>
>> Employ a medal tiffany bracelet <{snip}> a is
> {snip}
>
> So, apparently git-daemon's http features are being used by spammers.
Not at all. You appear to be referring to the message from
http://git.661346.n2.nabble.com/tiffany-bracelet-On-your-Significant-other-td7575440.html
This isn't a running instance of git-daemon, it's a web front-end for
the mailing list. It seems nabble allows image-attachments, and that's
what you're seeing; an attached image to a spam-email that was sent to
the git-mailing list through nabble.
The message contains HTML to display the image, and the git mailing
list rejects HTML messages. So the only ones who should be able to get
these spam-emails are users who subscribe through nabble. If you
subscribe through vger instead
(http://vger.kernel.org/vger-lists.html#git), you should get less
spam.
> In most cases, spam filters will correctly identify this junk.
>
> I wonder if there is a better way... In my mental sandbox, git-daemon
> http could have a set of deny/allow rules for incoming connection
> client types.
> e.g.:
>
> git: allow
> git-http: allow
> thunderbird: deny
> outlook express: replace linked file with rickroll.jpg
>
> and so on.. An out-of-the-box install probably should default to
> allow all to keep backward compatibility.
>
Git-daemon doesn't have an http-feature. You are probably thinking
about git-http-backend, but that's an CGI; the http-daemon invoking it
should already be able to filter connections. So, I don't think
there's anything that needs to be done to be able to block spammers
from git-servers. Blocking spammers from nabble is a different manner,
and is something you'll have to take up with the nabble staff.
^ permalink raw reply
* Re: [PATCH 1/1] Introduce new build variables INSTALL_MODE_EXECUTABLE and INSTALL_MODE_DATA.
From: Jeff Epler @ 2013-01-31 13:25 UTC (permalink / raw)
To: TJ; +Cc: git
In-Reply-To: <5109D230.2030101@iam.tj>
I was not familiar with this behavior of 'install -d' that it tries to change
the mode of an existing directory, but GNU coreutils 8.12.197-032bb
certainly behaves as TJ reports.
As a possible alternative, what about
[ -d $(DESTDIR)$(main1dir) ] || $(INSTALL) -d -m 755 $(DESTDIR)$(man1dir)
so that $(INSTALL) is not called when the target directory exists
already.
Jeff
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Ted Zlatanov @ 2013-01-31 15:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Michal Nazarewicz, git, Krzysztof Mazur,
Michal Nazarewicz
In-Reply-To: <7v7gmumzo6.fsf@alter.siamese.dyndns.org>
On Wed, 30 Jan 2013 07:57:29 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> Jeff King <peff@peff.net> writes:
>> But it would probably make sense for send-email to support the existing
>> git-credential subsystem, so that it can take advantage of secure
>> system-specific storage. And that is where we should be pointing new
>> users. I think contrib/mw-to-git even has credential support written in
>> perl, so it would just need to be factored out to Git.pm.
JCH> Yeah, that sounds like a neat idea.
Jeff, is there a way for git-credential to currently support
authinfo/netrc parsing? I assume that's the right way, instead of using
Michal's proposal to parse internally?
I'd like to add that, plus support for the 'string' and "string"
formats, and authinfo.gpg decoding through GPG. I'd write it in Perl,
if there's a choice.
Ted
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox