Re: repository corruption when pushing commits to a repository running 'git gc --prune='

[Matthieu Moy]

Jan Smets  writes:

> Hi
>
> I've recently expired my reflog to prune loose objects. On a live,
> bare, repository I ran 'git gc --prune=now'
>
> All clients ended up having problems, they would report:
>  error: refs/heads/master does not point to a valid object!
> Running 'git log' on the bare repo gave : fatal: bad object HEAD
[...]
> fatal: bad object 22f0351258fa0bb4cd28984b6473510957fbce69
> fatal: bad object 22f0351258fa0bb4cd28984b6473510957fbce69
> To /tmp/test/bare
>  ! [remote rejected] master -> master (missing necessary objects)

I think this is the expected behavior. push will create new objects that
are not referenced until the ref is updated (at the very end). prune can
run concurrently since creating and deleting objects is done in a
lockless way (only the ref update needs a lock).

Still, this is not the *documented* behavior, and an easy way to corrupt
a repo should be very explicitly documented as very dangerous, and the
precautions to take when using it should be explained clearly.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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

Alternative to manual editing with git add --patch

[Sven Helmberger]

Hello,

I hope this hasn't been discussed before.

I'm a big fan of cleanliness in commits and therefore often use git add
--patch to sort code changes I made into the right commits etc.

What I then often encountered was the situation where I happened to have
inserted consecutive lines of code that conceptually belong to different
commits. Normally I can nicely split patches, but not in this case,
making manually editing the patch the only alternative.

Shouldn't there be at least a way to quickly say line-by-line if you
want to have it added or not?

Personally, I find manually editing just annoying, it seems overly
arcane, but it also prevents me from really recommending "add --patch"
as best practice. I think it's a really good idea for many reasons to do
so, but I can't really tell people already struggling with using git
that I expect them to edit patches manually.

Regards,
Sven Helmberger
--
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: [BUG] Broken links in Git Documentation/user-manual.txt

[Matthieu Moy]

John Keeping  writes:

> On Wed, Oct 14, 2015 at 09:37:05AM +0200, Matthieu Moy wrote:
>> Xue Fuqiao  writes:
>> 
>> > Hi list,
>> >
>> > In https://git-scm.com/docs/user-manual.html , all links to the
>> > glossary[1] are broken.
>> 
>> Actually, the links themselves are fine, but the destimation is broken.
>> 
>> The doc is supposed to look like this :
>> 
>>   https://www.kernel.org/pub/software/scm/git/docs/user-manual.html#def_head
>> 
>> with the glossary at the end. On
>> https://git-scm.com/docs/user-manual.html, the glossary is displayed as
>> verbatim text.
>> 
>> This does not seem to be a bug in our user-manual.txt, but in the way
>> it's processed by git-scm.com.
>
> I think it was an issue in the source, but was fixed by be510e0
> (Documentation: fix section header mark-up, 2015-09-25).  I'm not sure
> when/how git-scm.com rebulds its documentation, but I'm pretty sure that
> fix hasn't made it into a release yet so I doubt the site has picked it
> up.

Ah, OK. I can't reproduce the issue even with versions earlier than
be510e0, but this is because my local build uses asciidoc, and the
commit message says "Asciidoctor is stricter than AsciiDoc", and
git-scm.com seems to use asciidoctor.

The fix isn't in any release (not even in master), so it can't be in
git-scm.com.

Thanks for the pointer, I'll update the GitHub bug.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: repository corruption when pushing commits to a repository running 'git gc --prune='

[Matthieu Moy]

Junio C Hamano  writes:

> diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
> index 5223498..fa15104 100644
> --- a/Documentation/git-gc.txt
> +++ b/Documentation/git-gc.txt
> @@ -63,8 +63,11 @@ automatic consolidation of packs.
>  --prune=::
>   Prune loose objects older than date (default is 2 weeks ago,
>   overridable by the config variable `gc.pruneExpire`).
> - --prune=all prunes loose objects regardless of their age.
> - --prune is on by default.
> + --prune=all prunes loose objects regardless of their age (do
> + not use --prune=all unless you know exactly what you are doing.
> + Unless the repository is quiescent, you will lose newly created
> + objects that haven't been anchored with the refs and end up
> + corrupting your repository).  --prune is on by default.

Looks good to me. I think the same should be added to git-prune.txt
under --expire.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: Alternative to manual editing with git add --patch

[Johannes Schindelin]

Hi Sven,

On 2015-10-14 17:07, Sven Helmberger wrote:

> What I then often encountered was the situation where I happened to have
> inserted consecutive lines of code that conceptually belong to different
> commits. Normally I can nicely split patches, but not in this case,
> making manually editing the patch the only alternative.

How about implementing it and then discussing the implementation? That would 
have two advantages:

1) there would be something tangential to discuss, and
2) even if Junio rejects it, you can carry the patch in your private fork and 
use it forever.

Ciao,
Johannes
--
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: repository corruption when pushing commits to a repository running 'git gc --prune='

[Matthieu Moy]

Jan Smets  writes:

> 2)
>
> remote: error: unable to write sha1 filename
> objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed: No such file or
> directory
> remote: fatal: failed to write object

If I read correctly, this happens when we move the temp file to its
permanent position.

This looks like a race between deleting objects/05/ and creating
objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed.

I don't understand how this is possible, since the temporary file and
the final one are in the same directory, so deleting the directory
should fail.

What am I missing?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] contrib: Add git-candidate subcommand

[Richard Ipsum]

git-candidate provides candidate review and patch tracking,
allowing distributed comment and review facilities with
all content stored in git.

Signed-off-by: Richard Ipsum 
---
 contrib/git-candidate/GitUtils.pm|  215 +++
 contrib/git-candidate/git-candidate.perl | 2602 ++
 2 files changed, 2817 insertions(+)
 create mode 100644 contrib/git-candidate/GitUtils.pm
 create mode 100755 contrib/git-candidate/git-candidate.perl

diff --git a/contrib/git-candidate/GitUtils.pm 
b/contrib/git-candidate/GitUtils.pm
new file mode 100644
index 000..6af7500
--- /dev/null
+++ b/contrib/git-candidate/GitUtils.pm
@@ -0,0 +1,215 @@
+# Copyright 2015 Codethink Limited
+#
+# GPL v2 (See COPYING)
+
+package GitUtils;
+
+use Exporter 'import';
+use Error qw(:try);
+use Git;
+
+our @EXPORT_OK = qw(require_clean_work_tree git_editor _head_symbolic_ref
+ _command_input_pipe_capture_output_oneline cat_file
+ is_object_present is_commit_merged get_head_branch
+ get_head_commit get_land_for_commit get_sha_at_ref
+ get_tracking_branch remove_refs inject_json
+ looks_like_sha validate_kwargs);
+
+# TODO: this really isn't the right place for this...
+use Carp;
+sub validate_kwargs
+{
+my ($kwargs_ref, @mandatory_args) = @_;
+
+for my $k (@mandatory_args) {
+confess "No '$k' provided" unless exists $kwargs_ref->{$k};
+}
+}
+
+sub looks_like_sha
+{
+my $x = shift;
+return $x =~ /[0-9a-z]{40}/;
+}
+
+sub inject_json
+{
+my ($repo, $json) = @_;
+my $fh = File::Temp->new();
+
+print $fh $json;
+close $fh;
+my $hash = $repo->hash_and_insert_object($fh->filename);
+
+return $hash;
+}
+
+sub require_clean_work_tree {
+my ($repo, $action) = @_;
+$action //= 'proceed';
+my $errno = 0;
+
+try {
+$repo->command_oneline('diff-files', '--quiet', '--ignore-submodules');
+}
+catch Git::Error::Command with {
+print STDERR "Cannot $action: You have unstaged changes.\n";
+$errno = 1;
+};
+
+try {
+$repo->command('diff-index', '--cached', '--quiet',
+'--ignore-submodules', 'HEAD');
+}
+catch Git::Error::Command with {
+if ($errno == 0) {
+print STDERR "Cannot $action: Your index contains uncommitted 
changes.\n";
+$errno = 1;
+}
+else {
+print STDERR "Additonally, your index contains uncommitted 
changes.\n";
+}
+};
+
+exit 1 if $errno;
+}
+
+# To Git.pm?
+sub git_editor
+{
+my $editor = $ENV{GIT_EDITOR} // `git var GIT_EDITOR`;
+chomp($editor);
+
+system($editor, @_);
+}
+
+# To Git.pm?
+sub _head_symbolic_ref
+{
+my $repo = shift;
+return $repo->command_oneline(['symbolic-ref', 'HEAD'], STDERR => 0);
+}
+
+# To Git.pm
+sub _command_input_pipe_capture_output_oneline
+{
+my ($repo, $command_input, $command, @arguments) = @_;
+
+confess "No repo defined" unless defined $repo;
+confess "No command input defined" unless defined $command_input;
+confess "No command defined" unless defined $command;
+
+my ($pid, $pipe_out, $pipe_in, $ctx) = $repo->command_bidi_pipe($command,
+@arguments);
+print $pipe_in $command_input;
+close $pipe_in;
+
+my $output = readline $pipe_out;
+chomp($output) if $output;
+
+$repo->command_close_bidi_pipe($pid, $pipe_out, undef, $ctx);
+
+return $output;
+}
+
+# TODO: To Git.pm? Make this similar to the cat_blob interface
+sub cat_file
+{
+my ($repo, $sha, $filename, %kwargs) = @_;
+return $repo->command(['cat-file', '-p',
+defined $filename ? "$sha:$filename" : "$sha"], %kwargs);
+}
+
+sub is_object_present
+{
+my ($repo, $sha) = @_;
+
+return try {
+my $output = $repo->command_oneline(['cat-file', '-t', $sha],
+STDERR => 0);
+return 1;
+}
+catch Git::Error::Command with {
+return 0;
+};
+}
+
+sub is_commit_merged
+{
+my ($repo, $what, $where) = @_;
+my @lines = $repo->command('rev-list', $where);
+return grep { /$what/ } @lines;
+}
+
+sub get_head_branch
+{
+my $repo = shift;
+
+return try {
+my $symbolic_ref = _head_symbolic_ref($repo);
+return $repo->command_oneline('for-each-ref',
+'--format=%(refname:short)', $symbolic_ref);
+}
+catch Git::Error::Command with {
+return undef;
+};
+}
+
+sub get_head_commit
+{
+my $repo = shift;
+
+return try {
+return $repo->command_oneline('rev-parse', 'HEAD');
+}
+catch Git::Error::Command with {
+return undef;
+};
+}
+
+# TODO: This function will be useful if we want to handle tracking
+# branches, right now I'd rather not bother with that.
+sub get_land_for_commit
+{
+my ($repo, $commit) = @_;
+
+return try {
+my $symbolic_ref = _head_symbolic_ref($repo);
+my $line 

[RFC] Git based patch tracking and review

[Richard Ipsum]

Hi,

I've been working to add patch tracking and candidate
review to git. The motivation for this is simply that most patch
tracking systems are centralised, since git is a distributed vcs it
seems appropriate that patches, reviews and comments should also
be distributed.

The system aims to be fairly minimal, it adds a candidate
subcommand which can be used to manipulate the state of candidates:
add reviews, comment, submit new versions etc. This prototype is,
as you would probably expect, by no means complete, at this point
it seems useful to stop and ask where we should take this next.

git-candidate is written in perl, with the hope that in time it
can potentially be included within contrib.

The original source repository for this project can be found
at https://bitbucket.org/richardipsum/git-candidate

Richard Ipsum (2):
  contrib: Add git-candidate subcommand
  contrib/git-candidate: Add README

 contrib/git-candidate/GitUtils.pm|  215 +++
 contrib/git-candidate/README.md  |  153 ++
 contrib/git-candidate/git-candidate.perl | 2602 ++
 3 files changed, 2970 insertions(+)
 create mode 100644 contrib/git-candidate/GitUtils.pm
 create mode 100644 contrib/git-candidate/README.md
 create mode 100755 contrib/git-candidate/git-candidate.perl

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] contrib/git-candidate: Add README

[Richard Ipsum]

Describes motivation for git-candidate and shows an example workflow.

Signed-off-by: Richard Ipsum 
---
 contrib/git-candidate/README.md | 153 
 1 file changed, 153 insertions(+)
 create mode 100644 contrib/git-candidate/README.md

diff --git a/contrib/git-candidate/README.md b/contrib/git-candidate/README.md
new file mode 100644
index 000..3291a12
--- /dev/null
+++ b/contrib/git-candidate/README.md
@@ -0,0 +1,153 @@
+git-candidate
+=
+
+git-candidate provides candidate review and patch tracking,
+it differs from other tools that provide this by storing _all_
+content within git.
+
+## Why?
+
+Existing tools such as Github's pull-requests and Gerrit are already
+in wide use, why bother with something new?
+
+We are concerned that whilst git is a distributed version control
+system the systems used to store comments and reviews for content
+under version control are usually centralised,
+git-candidate aims to solve this by storing
+all patch-tracking data in git proper.
+
+## Example review process
+
+### Contributor - Submits a candidate
+
+   (hack hack hack)
+
+   (feature)$ git commit -m "Add archived repo"
+   (feature)$ git candidate create archivedrepo master
+   -m "Add support for archived repo"
+   Candidate archivedrepo created successfully.
+   (feature)$ git candidate submit archivedrepo origin
+   Candidate was submitted successfully.
+
+### Upstream - Reviews candidate
+
+   (master)$ git candidate fetch origin
+   (master)$ git candidate status archiverepo origin
+   Revision: 6239bd72d597357af901718becae91cee2a32b73
+   Ref: candidates/origin/archiverepo
+   Status: active
+   Land: master
+
+   Add archived repo support
+
+lib/gitano/command.lua | 28 ++--
+1 file changed, 22 insertions(+), 6 deletions(-)
+
+   (master)$ git show candidates/origin/archiverepo
+   commit 2db28539c8fa7b81122382bcc526c6706c9e113a
+   Author: Richard Ipsum 
+   Date:   Thu Oct 8 10:43:22 2015 +0100
+
+   Add support for archived repository masking in `ls`
+
+   By setting `project.archived` to something truthy, a repository
+   is thusly masked from `ls` output unless --all is passed in.
+
+   Signed-off-by: Richard Ipsum 
+   
+   
+
+
+   (master)$ git candidate review archiverepo origin --vote -1
+   -m "Sorry, I'll need to see tests before I can accept this"
+   Review added successfully
+
+### Contributor - Revises candidate
+
+   (master)$ git candidate fetch origin
+   (master)$ git candidate status archiverepo origin
+   Revision: 6239bd72d597357af901718becae91cee2a32b73
+   Ref: candidates/origin/archiverepo
+   Status: active
+   Land: master
+
+   Add archived repo support
+
+lib/gitano/command.lua | 28 ++--
+1 file changed, 22 insertions(+), 6 deletions(-)
+
+   

+   1 review
+   

+
+   Author: Emmet Hikory 
+   Date:   Tue Oct 13 10:09:45 2015 +0100
+   Vote:   -1
+
+   Sorry, I'll need to see tests before I can accept this
+
+   

+
+   (hack hack hack add tests)
+
+   (feature_v2)$ git log --oneline -1
+   Ensure the `ls` yarn checks for archived repos
+
+   (feature_v2)$ git candidate revise archiverepo origin
+   -m "Add archived repo support with tests"
+   Candidate archiverepo revised successfully.
+
+   (feature_v2)$ git candidate submit archiverepo origin
+   Candidate was submitted successfully.
+
+### Upstream - Merges candidate
+
+   (master)$ git candidate fetch origin
+   (master)$ git candidate status archiverepo origin
+   Revision: 4cd3d1197d399005a713ca55f126a9086356a072
+   Ref: candidates/origin/archiverepo
+   Status: active
+   Land: master
+
+   Add archived repo support with tests
+
+lib/gitano/command.lua  | 28 ++--
+testing/02-commands-ls.yarn | 19 +++
+2 files changed, 41 insertions(+), 6 deletions(-)
+
+   (master)$ git candidate review archiverepo origin --vote +2
+   -m "Looks good, merging.  Thanks for your efforts"
+   Review added successfully
+
+   (master)$ git candidate submit archiverepo origin
+   Candidate was submitted successfully.
+
+   (master)$ git merge candidates/origin/archiverepo
+   (master)$ git push origin master
+
+### Contributor - Observes candidate has been accepted
+
+   

Re: Alternative to manual editing with git add --patch

[Junio C Hamano]

Sven Helmberger  writes:

> I hope this hasn't been discussed before.
>
> I'm a big fan of cleanliness in commits and therefore often use git add
> --patch to sort code changes I made into the right commits etc.
>
> What I then often encountered was the situation where I happened to have
> inserted consecutive lines of code that conceptually belong to different
> commits. Normally I can nicely split patches, but not in this case,
> making manually editing the patch the only alternative.
>
> Shouldn't there be at least a way to quickly say line-by-line if you
> want to have it added or not?

I think this has been discussed a few times (and you should actually
hope that is the case---it shows that something that allows you to
split a hunk that consists of consecutive lines is not an obscure
and useless feature wish).  But I do not think we saw anybody came
up with a convincingly usable design (not the code design but how
the user interaction specifies where the hunk is cut in the first
place), let alone a prototype for it, so that we can discuss
further.  At least not yet.

As a quick-and-dirty change, you could invent a new variant of
's'plit that breaks a N-line hunk into N hunks with 1-line each, but
obviously that would not be a pleasant-enough UI to be called usable
when you have a hunk that adds 100 lines.  Perhaps "Split this hunk
into two by ending the earlier one immediately before the line that
has this substring" or something might be an idea?



--
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: Alternative to manual editing with git add --patch

[Matthieu Moy]

Sven Helmberger  writes:

> Hello,
>
> I hope this hasn't been discussed before.
>
> I'm a big fan of cleanliness in commits and therefore often use git add
> --patch to sort code changes I made into the right commits etc.
>
> What I then often encountered was the situation where I happened to have
> inserted consecutive lines of code that conceptually belong to different
> commits. Normally I can nicely split patches, but not in this case,
> making manually editing the patch the only alternative.
>
> Shouldn't there be at least a way to quickly say line-by-line if you
> want to have it added or not?

Many GUI or text-editor plugins for Git allow you to just select lines
and stage the selection. Even though I love "git add -p", I find magit
(Git integration for Emacs) more convenient when it comes to staging
individual lines.

That said, a "split hunk line by line" option for "git add -p" could be
nice.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: repository corruption when pushing commits to a repository running 'git gc --prune='

[Junio C Hamano]

Matthieu Moy  writes:

> I think this is the expected behavior. push will create new objects that
> are not referenced until the ref is updated (at the very end). prune can
> run concurrently since creating and deleting objects is done in a
> lockless way (only the ref update needs a lock).
>
> Still, this is not the *documented* behavior, and an easy way to corrupt
> a repo should be very explicitly documented as very dangerous, and the
> precautions to take when using it should be explained clearly.

Yeah, I think this paragraph in the user-manual is the only thing
that mentions it:

Anyway, once you are sure that you're not interested in any dangling
state, you can just prune all unreachable objects:


$ git prune


and they'll be gone. (You should only run `git prune` on a quiescent
repository--it's kind of like doing a filesystem fsck recovery: you
don't want to do that while the filesystem is mounted.

Something along this line, perhaps?

 Documentation/git-gc.txt | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 5223498..fa15104 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -63,8 +63,11 @@ automatic consolidation of packs.
 --prune=::
Prune loose objects older than date (default is 2 weeks ago,
overridable by the config variable `gc.pruneExpire`).
-   --prune=all prunes loose objects regardless of their age.
-   --prune is on by default.
+   --prune=all prunes loose objects regardless of their age (do
+   not use --prune=all unless you know exactly what you are doing.
+   Unless the repository is quiescent, you will lose newly created
+   objects that haven't been anchored with the refs and end up
+   corrupting your repository).  --prune is on by default.
 
 --no-prune::
Do not prune any loose objects.
--
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 08/10] ref-filter: add support for %(upstream:track,nobracket)

[Karthik Nayak]

 Wed, Oct 14, 2015 at 12:24 AM, Matthieu Moy
 wrote:
> Yes, but I still think that this was a bad idea. If you want nobracket
> to apply to "track", then the syntax should be
> %(upstream:track=nobracket). I think the "nobracket" should apply to
> "upstream" (i.e. be global to the atom), hence
> %(upstream:nobracket,track) and %(upstream:track,nobracket) should both
> be accepted. Possibly %(upstream:,nobracket) could complain,
> but just ignoring "nobracket" in this case would make sense to me.
>

Oh okay, was thinking only WRT the "track" option.

> Special-casing the implementation of "nobracket" also means you're
> special-casing its user-visible behavior. And as a user, I hate it when
> the behavior is subtely different for things that look like each other
> (in this case, %(align:...) and %(upstream:...) ).
>

Makes sense, was just looking for opinions.

>> %(upstream:nobracket,track) to be supported then, I think we'll have
>> to change this whole layout and have the detection done up where we
>> locat "upstream" / "push", what would be a nice way to go around this?
>
> You mean, below
>
> else if (starts_with(name, "upstream")) {
>
> within populate_value()?
>
> I think it would, yes.
>

Yes, that's what I meant.

>> What I could think of:
>> 1. Do the cleanup that Junio and Matthieu suggested, where we
>> basically parse the
>> atoms and store them into a used_atom struct. I could either work on
>> those patches
>> before this and then rebase this on top.
>> 2. Let this be and come back on it when implementing the above series.
>> After reading Matthieu's and Junio's discussion, I lean towards the latter.
>
> Leaving it as-is does not fit in my arguments to do the refactoring
> later. It's not introducing "another instance of an existing pattern",
> but actually a new pattern.
>

I meant after changing whatever we discussed above.

-- 
Regards,
Karthik Nayak
--
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: [BUG] Broken links in Git Documentation/user-manual.txt

[Matthieu Moy]

Xue Fuqiao  writes:

> Hi list,
>
> In https://git-scm.com/docs/user-manual.html , all links to the
> glossary[1] are broken.

Actually, the links themselves are fine, but the destimation is broken.

The doc is supposed to look like this :

  https://www.kernel.org/pub/software/scm/git/docs/user-manual.html#def_head

with the glossary at the end. On
https://git-scm.com/docs/user-manual.html, the glossary is displayed as
verbatim text.

This does not seem to be a bug in our user-manual.txt, but in the way
it's processed by git-scm.com.

I reported the issue there:

https://github.com/git/git-scm.com/issues/605

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] send-email: allow to compose only prepared cover letter

[Andy Shevchenko]

On Tue, 2015-10-13 at 14:11 -0700, Junio C Hamano wrote:
> Andy Shevchenko  writes:
> 
> > My often use case is to do:
> > % git format-patch --cover-letter --subject-prefix="PATCH 
> > vN" rev1^..revXYZ
> > % $GIT_EDITOR -*
> > % git send-email 00* # assumes series less than 100 
> > patches
> > % rm -f 00*
> 
> I guess this patch would not hurt too much, but the above would
> vastly be improved if you used "-vN" option, instead of the
> hand-rolled subject prefix, and dropped the last "rm -f" (which in
> turn would mean you would want to use -o option to specify where to
> keep the older iterations of the topic).  Then you can easily refer
> to cover letters and patches from previous rounds while preparing
> the latest to send out.

Thanks!

The patch I sent was a modification of previous variant I have. It
unfortunately misses undef $compose; line in the first conditional
branch.

Besides that I found that --annotate might be a workaround and it comes
a burden of editor how to avoid editing every patch but first.

-- 
Andy Shevchenko 
Intel Finland Oy
--
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

[ANNOUNCE] Git Rev News edition 8

[Christian Couder]

Hi everyone,

I'm happy announce that the 8th edition of Git Rev News is now published:

https://git.github.io/rev_news/2015/10/14/edition-8/

Thanks a lot to all the contributors!

Enjoy,
Christian, Thomas and Nicola.
--
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

Captain Kelvin Ken Miller

Am Captain Kelvin Ken Miller currently I need you assistant to move some funds 
out of Iraq. Kindly respond for more details.
--
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 02/26] mailinfo: fix for off-by-one error in boundary stack

[Stefan Beller]

On Tue, Oct 13, 2015 at 4:16 PM, Junio C Hamano  wrote:
> We pre-increment the pointer that we will use to store something at,
> so the pointer is already beyond the end of the array if it points
> at content[MAX_BOUNDARIES].
>
> Signed-off-by: Junio C Hamano 
> ---
>
>  As always, I am very bad at checking and fixing off-by-one errors.
>  A few extra sets of eyeballs are very much appreciated.

some relevant lines from the file:
#define MAX_BOUNDARIES 5
static struct strbuf *content[MAX_BOUNDARIES];
static struct strbuf **content_top = content;

content has slots 0..4, so checking content[5] is out of range,
but was allowed in the original code.

So this patch looks good to me, though looking at
289796dd29dd656734cfd59b657deb943a71cf6a,
makes me wonder if we forget the other spot where it's
decremented in that patch.

Also looking at the patch, makes we wonder if we rather want to change
-static struct strbuf *content[MAX_BOUNDARIES];
+static struct strbuf *content[MAX_BOUNDARIES + 1];

instead? That would actually increase the allocated memory,
and allow to write one more content line, but reading the code is
easier as we don't need to reason about > or >=.

Another way would be to rewrite this part to use an index instead
of content_top being a pointer. And the index could be compared
to strictly less than MAX_BOUNDARIES.

> ---
>  builtin/mailinfo.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
> index 330bef4..2742d0d 100644
> --- a/builtin/mailinfo.c
> +++ b/builtin/mailinfo.c
> @@ -185,7 +185,7 @@ static void handle_content_type(struct strbuf *line)
>
> if (slurp_attr(line->buf, "boundary=", boundary)) {
> strbuf_insert(boundary, 0, "--", 2);
> -   if (++content_top > [MAX_BOUNDARIES]) {
> +   if (++content_top >= [MAX_BOUNDARIES]) {
> fprintf(stderr, "Too many boundaries to handle\n");
> exit(1);
> }
> --
> 2.6.1-320-g86a1181
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/26] mailinfo: fix for off-by-one error in boundary stack

[Junio C Hamano]

I'll be sending out v2 very soon, so you might want to hold off for
a few minutes.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 17/31] mailinfo: move transfer_encoding to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 00b8b4b..140b31ee 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -23,14 +23,14 @@ struct mailinfo {
const char *metainfo_charset;
 
char *message_id;
+   enum  {
+   TE_DONTCARE, TE_QP, TE_BASE64
+   } transfer_encoding;
int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
 };
 
-static enum  {
-   TE_DONTCARE, TE_QP, TE_BASE64
-} transfer_encoding;
 
 static struct strbuf charset = STRBUF_INIT;
 
@@ -214,14 +214,15 @@ static void handle_message_id(struct mailinfo *mi, const 
struct strbuf *line)
mi->message_id = strdup(line->buf);
 }
 
-static void handle_content_transfer_encoding(const struct strbuf *line)
+static void handle_content_transfer_encoding(struct mailinfo *mi,
+const struct strbuf *line)
 {
if (strcasestr(line->buf, "base64"))
-   transfer_encoding = TE_BASE64;
+   mi->transfer_encoding = TE_BASE64;
else if (strcasestr(line->buf, "quoted-printable"))
-   transfer_encoding = TE_QP;
+   mi->transfer_encoding = TE_QP;
else
-   transfer_encoding = TE_DONTCARE;
+   mi->transfer_encoding = TE_DONTCARE;
 }
 
 static int is_multipart_boundary(const struct strbuf *line)
@@ -356,7 +357,7 @@ static int check_header(struct mailinfo *mi,
len = strlen("Content-Transfer-Encoding: ");
strbuf_add(, line->buf + len, line->len - len);
decode_header(mi, );
-   handle_content_transfer_encoding();
+   handle_content_transfer_encoding(mi, );
ret = 1;
goto check_header_out;
}
@@ -615,11 +616,11 @@ release_return:
strbuf_release();
 }
 
-static void decode_transfer_encoding(struct strbuf *line)
+static void decode_transfer_encoding(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf *ret;
 
-   switch (transfer_encoding) {
+   switch (mi->transfer_encoding) {
case TE_QP:
ret = decode_q_segment(line, 0);
break;
@@ -838,7 +839,7 @@ again:
}
 
/* set some defaults */
-   transfer_encoding = TE_DONTCARE;
+   mi->transfer_encoding = TE_DONTCARE;
strbuf_reset();
 
/* slurp in this section's info */
@@ -876,9 +877,9 @@ static void handle_body(struct mailinfo *mi, struct strbuf 
*line)
}
 
/* Unwrap transfer encoding */
-   decode_transfer_encoding(line);
+   decode_transfer_encoding(mi, line);
 
-   switch (transfer_encoding) {
+   switch (mi->transfer_encoding) {
case TE_BASE64:
case TE_QP:
{
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 12/31] mailinfo: move filter/header stage to struct mailinfo

[Junio C Hamano]

Earlier we got rid of two function-scope static variables that kept
track of the states of helper functions by making them extra arguments
that are passed throughout the callchain.  Now we have a convenient
place to store and pass them around in the form of "struct mailinfo",
change them into two fields in the struct.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 42 ++
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 68a085e..5d2d88a 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -19,6 +19,9 @@ struct mailinfo {
struct strbuf email;
int keep_subject;
int keep_non_patch_brackets_in_subject;
+
+   int filter_stage; /* still reading log or are we copying patch? */
+   int header_stage; /* still checking in-body headers? */
 };
 static char *message_id;
 
@@ -28,6 +31,7 @@ static enum  {
 
 static struct strbuf charset = STRBUF_INIT;
 static int patch_lines;
+
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
 static int add_message_id;
@@ -718,25 +722,25 @@ static int is_scissors_line(const struct strbuf *line)
gap * 2 < perforation);
 }
 
-static int handle_commit_msg(struct strbuf *line, int *still_looking)
+static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
 {
if (!cmitmsg)
return 0;
 
-   if (*still_looking) {
+   if (mi->header_stage) {
if (!line->len || (line->len == 1 && line->buf[0] == '\n'))
return 0;
}
 
-   if (use_inbody_headers && *still_looking) {
-   *still_looking = check_header(line, s_hdr_data, 0);
-   if (*still_looking)
+   if (use_inbody_headers && mi->header_stage) {
+   mi->header_stage = check_header(line, s_hdr_data, 0);
+   if (mi->header_stage)
return 0;
} else
/* Only trim the first (blank) line of the commit message
 * when ignoring in-body headers.
 */
-   *still_looking = 0;
+   mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
if (metainfo_charset)
@@ -748,7 +752,7 @@ static int handle_commit_msg(struct strbuf *line, int 
*still_looking)
die_errno("Could not rewind output message file");
if (ftruncate(fileno(cmitmsg), 0))
die_errno("Could not truncate output message file at 
scissors");
-   *still_looking = 1;
+   mi->header_stage = 1;
 
/*
 * We may have already read "secondary headers"; purge
@@ -780,13 +784,13 @@ static void handle_patch(const struct strbuf *line)
patch_lines++;
 }
 
-static void handle_filter(struct strbuf *line, int *filter_stage, int 
*header_stage)
+static void handle_filter(struct mailinfo *mi, struct strbuf *line)
 {
-   switch (*filter_stage) {
+   switch (mi->filter_stage) {
case 0:
-   if (!handle_commit_msg(line, header_stage))
+   if (!handle_commit_msg(mi, line))
break;
-   (*filter_stage)++;
+   mi->filter_stage++;
case 1:
handle_patch(line);
break;
@@ -802,8 +806,7 @@ static int find_boundary(struct mailinfo *mi, struct strbuf 
*line)
return 0;
 }
 
-static int handle_boundary(struct mailinfo *mi, struct strbuf *line,
-  int *filter_stage, int *header_stage)
+static int handle_boundary(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf newline = STRBUF_INIT;
 
@@ -825,7 +828,7 @@ again:
"can't recover\n");
exit(1);
}
-   handle_filter(, filter_stage, header_stage);
+   handle_filter(mi, );
strbuf_release();
 
/* skip to the next boundary */
@@ -853,8 +856,6 @@ again:
 static void handle_body(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf prev = STRBUF_INIT;
-   int filter_stage = 0;
-   int header_stage = 1;
 
/* Skip up to the first boundary */
if (*content_top) {
@@ -867,10 +868,10 @@ static void handle_body(struct mailinfo *mi, struct 
strbuf *line)
if (*content_top && is_multipart_boundary(line)) {
/* flush any leftover */
if (prev.len) {
-   handle_filter(, _stage, 
_stage);
+   handle_filter(mi, );
strbuf_reset();
}
-   if (!handle_boundary(mi, line, _stage, 
_stage))
+   if (!handle_boundary(mi, line))
goto 

[PATCH v2 28/31] mailinfo: libify

[Junio C Hamano]

Move the bulk of the code from builtin/mailinfo.c to mailinfo.c
so that new callers can start calling mailinfo() directly.

Note that a few calls to exit() and die() need to be cleaned up
for the API to be truly useful, which will come in later steps.

Signed-off-by: Junio C Hamano 
---
 Makefile   |1 +
 builtin/mailinfo.c | 1036 +---
 mailinfo.c | 1007 ++
 mailinfo.h |   40 ++
 4 files changed, 1049 insertions(+), 1035 deletions(-)
 create mode 100644 mailinfo.c
 create mode 100644 mailinfo.h

diff --git a/Makefile b/Makefile
index 8d5df7e..7dd3bff 100644
--- a/Makefile
+++ b/Makefile
@@ -726,6 +726,7 @@ LIB_OBJS += list-objects.o
 LIB_OBJS += ll-merge.o
 LIB_OBJS += lockfile.o
 LIB_OBJS += log-tree.o
+LIB_OBJS += mailinfo.o
 LIB_OBJS += mailmap.o
 LIB_OBJS += match-trees.o
 LIB_OBJS += merge.o
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index de446ec..f6df274 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -6,1041 +6,7 @@
 #include "builtin.h"
 #include "utf8.h"
 #include "strbuf.h"
-
-#define MAX_BOUNDARIES 5
-
-struct mailinfo {
-   FILE *input;
-   FILE *output;
-   FILE *patchfile;
-
-   struct strbuf name;
-   struct strbuf email;
-   int keep_subject;
-   int keep_non_patch_brackets_in_subject;
-   int add_message_id;
-   int use_scissors;
-   int use_inbody_headers; /* defaults to 1 */
-   const char *metainfo_charset;
-
-   struct strbuf *content[MAX_BOUNDARIES];
-   struct strbuf **content_top;
-   struct strbuf charset;
-   char *message_id;
-   enum  {
-   TE_DONTCARE, TE_QP, TE_BASE64
-   } transfer_encoding;
-   int patch_lines;
-   int filter_stage; /* still reading log or are we copying patch? */
-   int header_stage; /* still checking in-body headers? */
-   struct strbuf **p_hdr_data;
-   struct strbuf **s_hdr_data;
-
-   struct strbuf log_message;
-};
-
-static void cleanup_space(struct strbuf *sb)
-{
-   size_t pos, cnt;
-   for (pos = 0; pos < sb->len; pos++) {
-   if (isspace(sb->buf[pos])) {
-   sb->buf[pos] = ' ';
-   for (cnt = 0; isspace(sb->buf[pos + cnt + 1]); cnt++);
-   strbuf_remove(sb, pos + 1, cnt);
-   }
-   }
-}
-
-static void get_sane_name(struct strbuf *out, struct strbuf *name, struct 
strbuf *email)
-{
-   struct strbuf *src = name;
-   if (name->len < 3 || 60 < name->len || strchr(name->buf, '@') ||
-   strchr(name->buf, '<') || strchr(name->buf, '>'))
-   src = email;
-   else if (name == out)
-   return;
-   strbuf_reset(out);
-   strbuf_addbuf(out, src);
-}
-
-static void parse_bogus_from(struct mailinfo *mi, const struct strbuf *line)
-{
-   /* John Doe  */
-
-   char *bra, *ket;
-   /* This is fallback, so do not bother if we already have an
-* e-mail address.
-*/
-   if (mi->email.len)
-   return;
-
-   bra = strchr(line->buf, '<');
-   if (!bra)
-   return;
-   ket = strchr(bra, '>');
-   if (!ket)
-   return;
-
-   strbuf_reset(>email);
-   strbuf_add(>email, bra + 1, ket - bra - 1);
-
-   strbuf_reset(>name);
-   strbuf_add(>name, line->buf, bra - line->buf);
-   strbuf_trim(>name);
-   get_sane_name(>name, >name, >email);
-}
-
-static void handle_from(struct mailinfo *mi, const struct strbuf *from)
-{
-   char *at;
-   size_t el;
-   struct strbuf f;
-
-   strbuf_init(, from->len);
-   strbuf_addbuf(, from);
-
-   at = strchr(f.buf, '@');
-   if (!at) {
-   parse_bogus_from(mi, from);
-   return;
-   }
-
-   /*
-* If we already have one email, don't take any confusing lines
-*/
-   if (mi->email.len && strchr(at + 1, '@')) {
-   strbuf_release();
-   return;
-   }
-
-   /* Pick up the string around '@', possibly delimited with <>
-* pair; that is the email part.
-*/
-   while (at > f.buf) {
-   char c = at[-1];
-   if (isspace(c))
-   break;
-   if (c == '<') {
-   at[-1] = ' ';
-   break;
-   }
-   at--;
-   }
-   el = strcspn(at, " \n\t\r\v\f>");
-   strbuf_reset(>email);
-   strbuf_add(>email, at, el);
-   strbuf_remove(, at - f.buf, el + (at[el] ? 1 : 0));
-
-   /* The remainder is name.  It could be
-*
-* - "John Doe "   (a), or
-* - "john.doe@xz (John Doe)"   (b), or
-* - "John (zzz) Doe  (Comment)"   (c)
-*
-* but we have removed the email part, so
-   

[PATCH v2 26/31] mailinfo: move cleanup_space() before its users

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index ee669b9..f4771ee 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -41,8 +41,17 @@ struct mailinfo {
 
 #define MAX_HDR_PARSED 10
 
-static void cleanup_space(struct strbuf *sb);
-
+static void cleanup_space(struct strbuf *sb)
+{
+   size_t pos, cnt;
+   for (pos = 0; pos < sb->len; pos++) {
+   if (isspace(sb->buf[pos])) {
+   sb->buf[pos] = ' ';
+   for (cnt = 0; isspace(sb->buf[pos + cnt + 1]); cnt++);
+   strbuf_remove(sb, pos + 1, cnt);
+   }
+   }
+}
 
 static void get_sane_name(struct strbuf *out, struct strbuf *name, struct 
strbuf *email)
 {
@@ -281,18 +290,6 @@ static void cleanup_subject(struct mailinfo *mi, struct 
strbuf *subject)
strbuf_trim(subject);
 }
 
-static void cleanup_space(struct strbuf *sb)
-{
-   size_t pos, cnt;
-   for (pos = 0; pos < sb->len; pos++) {
-   if (isspace(sb->buf[pos])) {
-   sb->buf[pos] = ' ';
-   for (cnt = 0; isspace(sb->buf[pos + cnt + 1]); cnt++);
-   strbuf_remove(sb, pos + 1, cnt);
-   }
-   }
-}
-
 static const char *header[MAX_HDR_PARSED] = {
"From","Subject","Date",
 };
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 16/31] mailinfo: move metainfo_charset to struct mailinfo

[Junio C Hamano]

This requires us to pass the struct down to decode_header() and
convert_to_utf8() callchain.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 40 +++-
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 80034a2..00b8b4b 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -9,8 +9,6 @@
 
 static FILE *cmitmsg, *patchfile;
 
-static const char *metainfo_charset;
-
 struct mailinfo {
FILE *input;
FILE *output;
@@ -22,6 +20,7 @@ struct mailinfo {
int add_message_id;
int use_scissors;
int use_inbody_headers; /* defaults to 1 */
+   const char *metainfo_charset;
 
char *message_id;
int patch_lines;
@@ -293,7 +292,7 @@ static void cleanup_space(struct strbuf *sb)
}
 }
 
-static void decode_header(struct strbuf *line);
+static void decode_header(struct mailinfo *mi, struct strbuf *line);
 static const char *header[MAX_HDR_PARSED] = {
"From","Subject","Date",
 };
@@ -336,7 +335,7 @@ static int check_header(struct mailinfo *mi,
 * normalize the meta information to utf8.
 */
strbuf_add(, line->buf + len + 2, line->len - len - 
2);
-   decode_header();
+   decode_header(mi, );
handle_header(_data[i], );
ret = 1;
goto check_header_out;
@@ -347,7 +346,7 @@ static int check_header(struct mailinfo *mi,
if (cmp_header(line, "Content-Type")) {
len = strlen("Content-Type: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header();
+   decode_header(mi, );
strbuf_insert(, 0, "Content-Type: ", len);
handle_content_type();
ret = 1;
@@ -356,7 +355,7 @@ static int check_header(struct mailinfo *mi,
if (cmp_header(line, "Content-Transfer-Encoding")) {
len = strlen("Content-Transfer-Encoding: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header();
+   decode_header(mi, );
handle_content_transfer_encoding();
ret = 1;
goto check_header_out;
@@ -364,7 +363,7 @@ static int check_header(struct mailinfo *mi,
if (cmp_header(line, "Message-Id")) {
len = strlen("Message-Id: ");
strbuf_add(, line->buf + len, line->len - len);
-   decode_header();
+   decode_header(mi, );
handle_message_id(mi, );
ret = 1;
goto check_header_out;
@@ -520,23 +519,24 @@ static struct strbuf *decode_b_segment(const struct 
strbuf *b_seg)
return out;
 }
 
-static void convert_to_utf8(struct strbuf *line, const char *charset)
+static void convert_to_utf8(struct mailinfo *mi,
+   struct strbuf *line, const char *charset)
 {
char *out;
 
-   if (!charset || !*charset)
+   if (!mi->metainfo_charset || !charset || !*charset)
return;
 
-   if (same_encoding(metainfo_charset, charset))
+   if (same_encoding(mi->metainfo_charset, charset))
return;
-   out = reencode_string(line->buf, metainfo_charset, charset);
+   out = reencode_string(line->buf, mi->metainfo_charset, charset);
if (!out)
die("cannot convert from %s to %s",
-   charset, metainfo_charset);
+   charset, mi->metainfo_charset);
strbuf_attach(line, out, strlen(out), strlen(out));
 }
 
-static void decode_header(struct strbuf *it)
+static void decode_header(struct mailinfo *mi, struct strbuf *it)
 {
char *in, *ep, *cp;
struct strbuf outbuf = STRBUF_INIT, *dec;
@@ -599,8 +599,7 @@ static void decode_header(struct strbuf *it)
dec = decode_q_segment(, 1);
break;
}
-   if (metainfo_charset)
-   convert_to_utf8(dec, charset_q.buf);
+   convert_to_utf8(mi, dec, charset_q.buf);
 
strbuf_addbuf(, dec);
strbuf_release(dec);
@@ -745,8 +744,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
-   if (metainfo_charset)
-   convert_to_utf8(line, charset.buf);
+   convert_to_utf8(mi, line, charset.buf);
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
@@ -1047,7 +1045,7 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
setup_mailinfo();
 
def_charset = get_commit_output_encoding();
-   metainfo_charset = def_charset;
+   mi.metainfo_charset = def_charset;
 

[PATCH v2 15/31] mailinfo: move use_scissors and use_inbody_headers to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 6703453..80034a2 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -20,6 +20,8 @@ struct mailinfo {
int keep_subject;
int keep_non_patch_brackets_in_subject;
int add_message_id;
+   int use_scissors;
+   int use_inbody_headers; /* defaults to 1 */
 
char *message_id;
int patch_lines;
@@ -34,8 +36,6 @@ static enum  {
 static struct strbuf charset = STRBUF_INIT;
 
 static struct strbuf **p_hdr_data, **s_hdr_data;
-static int use_scissors;
-static int use_inbody_headers = 1;
 
 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
@@ -734,7 +734,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
return 0;
}
 
-   if (use_inbody_headers && mi->header_stage) {
+   if (mi->use_inbody_headers && mi->header_stage) {
mi->header_stage = check_header(mi, line, s_hdr_data, 0);
if (mi->header_stage)
return 0;
@@ -748,7 +748,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
if (metainfo_charset)
convert_to_utf8(line, charset.buf);
 
-   if (use_scissors && is_scissors_line(line)) {
+   if (mi->use_scissors && is_scissors_line(line)) {
int i;
if (fseek(cmitmsg, 0L, SEEK_SET))
die_errno("Could not rewind output message file");
@@ -1009,12 +1009,14 @@ static int mailinfo(struct mailinfo *mi, const char 
*msg, const char *patch)
return 0;
 }
 
-static int git_mailinfo_config(const char *var, const char *value, void 
*unused)
+static int git_mailinfo_config(const char *var, const char *value, void *mi_)
 {
+   struct mailinfo *mi = mi_;
+
if (!starts_with(var, "mailinfo."))
-   return git_default_config(var, value, unused);
+   return git_default_config(var, value, NULL);
if (!strcmp(var, "mailinfo.scissors")) {
-   use_scissors = git_config_bool(var, value);
+   mi->use_scissors = git_config_bool(var, value);
return 0;
}
/* perhaps others here */
@@ -1027,6 +1029,7 @@ static void setup_mailinfo(struct mailinfo *mi)
strbuf_init(>name, 0);
strbuf_init(>email, 0);
mi->header_stage = 1;
+   mi->use_inbody_headers = 1;
git_config(git_mailinfo_config, );
 }
 
@@ -1060,11 +1063,11 @@ int cmd_mailinfo(int argc, const char **argv, const 
char *prefix)
else if (starts_with(argv[1], "--encoding="))
metainfo_charset = argv[1] + 11;
else if (!strcmp(argv[1], "--scissors"))
-   use_scissors = 1;
+   mi.use_scissors = 1;
else if (!strcmp(argv[1], "--no-scissors"))
-   use_scissors = 0;
+   mi.use_scissors = 0;
else if (!strcmp(argv[1], "--no-inbody-headers"))
-   use_inbody_headers = 0;
+   mi.use_inbody_headers = 0;
else
usage(mailinfo_usage);
argc--; argv++;
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 11/31] mailinfo: move global "FILE *fin, *fout" to struct mailinfo

[Junio C Hamano]

This requires us to pass "struct mailinfo" to more functions
throughout the codepath that read input lines, which makes
later steps easier.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 54 --
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 855d813..68a085e 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -7,11 +7,14 @@
 #include "utf8.h"
 #include "strbuf.h"
 
-static FILE *cmitmsg, *patchfile, *fin, *fout;
+static FILE *cmitmsg, *patchfile;
 
 static const char *metainfo_charset;
 
 struct mailinfo {
+   FILE *input;
+   FILE *output;
+
struct strbuf name;
struct strbuf email;
int keep_subject;
@@ -790,16 +793,17 @@ static void handle_filter(struct strbuf *line, int 
*filter_stage, int *header_st
}
 }
 
-static int find_boundary(struct strbuf *line)
+static int find_boundary(struct mailinfo *mi, struct strbuf *line)
 {
-   while (!strbuf_getline(line, fin, '\n')) {
+   while (!strbuf_getline(line, mi->input, '\n')) {
if (*content_top && is_multipart_boundary(line))
return 1;
}
return 0;
 }
 
-static int handle_boundary(struct strbuf *line, int *filter_stage, int 
*header_stage)
+static int handle_boundary(struct mailinfo *mi, struct strbuf *line,
+  int *filter_stage, int *header_stage)
 {
struct strbuf newline = STRBUF_INIT;
 
@@ -825,7 +829,7 @@ again:
strbuf_release();
 
/* skip to the next boundary */
-   if (!find_boundary(line))
+   if (!find_boundary(mi, line))
return 0;
goto again;
}
@@ -835,18 +839,18 @@ again:
strbuf_reset();
 
/* slurp in this section's info */
-   while (read_one_header_line(line, fin))
+   while (read_one_header_line(line, mi->input))
check_header(line, p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
-   if (strbuf_getline(line, fin, '\n'))
+   if (strbuf_getline(line, mi->input, '\n'))
return 0;
strbuf_addch(line, '\n');
return 1;
 }
 
-static void handle_body(struct strbuf *line)
+static void handle_body(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf prev = STRBUF_INIT;
int filter_stage = 0;
@@ -854,7 +858,7 @@ static void handle_body(struct strbuf *line)
 
/* Skip up to the first boundary */
if (*content_top) {
-   if (!find_boundary(line))
+   if (!find_boundary(mi, line))
goto handle_body_out;
}
 
@@ -866,7 +870,7 @@ static void handle_body(struct strbuf *line)
handle_filter(, _stage, 
_stage);
strbuf_reset();
}
-   if (!handle_boundary(line, _stage, 
_stage))
+   if (!handle_boundary(mi, line, _stage, 
_stage))
goto handle_body_out;
}
 
@@ -909,7 +913,7 @@ static void handle_body(struct strbuf *line)
handle_filter(line, _stage, _stage);
}
 
-   } while (!strbuf_getwholeline(line, fin, '\n'));
+   } while (!strbuf_getwholeline(line, mi->input, '\n'));
 
 handle_body_out:
strbuf_release();
@@ -951,29 +955,25 @@ static void handle_info(struct mailinfo *mi)
cleanup_subject(mi, hdr);
cleanup_space(hdr);
}
-   output_header_lines(fout, "Subject", hdr);
+   output_header_lines(mi->output, "Subject", hdr);
} else if (!strcmp(header[i], "From")) {
cleanup_space(hdr);
handle_from(mi, hdr);
-   fprintf(fout, "Author: %s\n", mi->name.buf);
-   fprintf(fout, "Email: %s\n", mi->email.buf);
+   fprintf(mi->output, "Author: %s\n", mi->name.buf);
+   fprintf(mi->output, "Email: %s\n", mi->email.buf);
} else {
cleanup_space(hdr);
-   fprintf(fout, "%s: %s\n", header[i], hdr->buf);
+   fprintf(mi->output, "%s: %s\n", header[i], hdr->buf);
}
}
-   fprintf(fout, "\n");
+   fprintf(mi->output, "\n");
 }
 
-static int mailinfo(struct mailinfo *mi,
-   FILE *in, FILE *out, const char *msg, const char *patch)
+static int mailinfo(struct mailinfo *mi, const char *msg, const char *patch)
 {
int peek;
struct strbuf line = STRBUF_INIT;
 
-   fin = in;
-   fout = out;
-
cmitmsg = fopen(msg, "w");
if (!cmitmsg) {
perror(msg);
@@ 

[PATCH v2 09/31] mailinfo: introduce "struct mailinfo" to hold globals

[Junio C Hamano]

Initially have only 'email' and 'name' fields in there and remove
the corresponding globals.  In subsequent patches, more globals will
be moved to this and the structure will be passed around as a new
parameter to more functions.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 61 +-
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index c8dc73f..9e41a10 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -12,8 +12,11 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
 static int keep_subject;
 static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
-static struct strbuf name = STRBUF_INIT;
-static struct strbuf email = STRBUF_INIT;
+
+struct mailinfo {
+   struct strbuf name;
+   struct strbuf email;
+};
 static char *message_id;
 
 static enum  {
@@ -45,7 +48,7 @@ static void get_sane_name(struct strbuf *out, struct strbuf 
*name, struct strbuf
strbuf_addbuf(out, src);
 }
 
-static void parse_bogus_from(const struct strbuf *line)
+static void parse_bogus_from(struct mailinfo *mi, const struct strbuf *line)
 {
/* John Doe  */
 
@@ -53,7 +56,7 @@ static void parse_bogus_from(const struct strbuf *line)
/* This is fallback, so do not bother if we already have an
 * e-mail address.
 */
-   if (email.len)
+   if (mi->email.len)
return;
 
bra = strchr(line->buf, '<');
@@ -63,16 +66,16 @@ static void parse_bogus_from(const struct strbuf *line)
if (!ket)
return;
 
-   strbuf_reset();
-   strbuf_add(, bra + 1, ket - bra - 1);
+   strbuf_reset(>email);
+   strbuf_add(>email, bra + 1, ket - bra - 1);
 
-   strbuf_reset();
-   strbuf_add(, line->buf, bra - line->buf);
-   strbuf_trim();
-   get_sane_name(, , );
+   strbuf_reset(>name);
+   strbuf_add(>name, line->buf, bra - line->buf);
+   strbuf_trim(>name);
+   get_sane_name(>name, >name, >email);
 }
 
-static void handle_from(const struct strbuf *from)
+static void handle_from(struct mailinfo *mi, const struct strbuf *from)
 {
char *at;
size_t el;
@@ -83,14 +86,14 @@ static void handle_from(const struct strbuf *from)
 
at = strchr(f.buf, '@');
if (!at) {
-   parse_bogus_from(from);
+   parse_bogus_from(mi, from);
return;
}
 
/*
 * If we already have one email, don't take any confusing lines
 */
-   if (email.len && strchr(at + 1, '@')) {
+   if (mi->email.len && strchr(at + 1, '@')) {
strbuf_release();
return;
}
@@ -109,8 +112,8 @@ static void handle_from(const struct strbuf *from)
at--;
}
el = strcspn(at, " \n\t\r\v\f>");
-   strbuf_reset();
-   strbuf_add(, at, el);
+   strbuf_reset(>email);
+   strbuf_add(>email, at, el);
strbuf_remove(, at - f.buf, el + (at[el] ? 1 : 0));
 
/* The remainder is name.  It could be
@@ -132,7 +135,7 @@ static void handle_from(const struct strbuf *from)
strbuf_setlen(, f.len - 1);
}
 
-   get_sane_name(, , );
+   get_sane_name(>name, , >email);
strbuf_release();
 }
 
@@ -929,7 +932,7 @@ static void output_header_lines(FILE *fout, const char 
*hdr, const struct strbuf
}
 }
 
-static void handle_info(void)
+static void handle_info(struct mailinfo *mi)
 {
struct strbuf *hdr;
int i;
@@ -951,9 +954,9 @@ static void handle_info(void)
output_header_lines(fout, "Subject", hdr);
} else if (!strcmp(header[i], "From")) {
cleanup_space(hdr);
-   handle_from(hdr);
-   fprintf(fout, "Author: %s\n", name.buf);
-   fprintf(fout, "Email: %s\n", email.buf);
+   handle_from(mi, hdr);
+   fprintf(fout, "Author: %s\n", mi->name.buf);
+   fprintf(fout, "Email: %s\n", mi->email.buf);
} else {
cleanup_space(hdr);
fprintf(fout, "%s: %s\n", header[i], hdr->buf);
@@ -962,7 +965,8 @@ static void handle_info(void)
fprintf(fout, "\n");
 }
 
-static int mailinfo(FILE *in, FILE *out, const char *msg, const char *patch)
+static int mailinfo(struct mailinfo *mi,
+   FILE *in, FILE *out, const char *msg, const char *patch)
 {
int peek;
struct strbuf line = STRBUF_INIT;
@@ -997,7 +1001,7 @@ static int mailinfo(FILE *in, FILE *out, const char *msg, 
const char *patch)
handle_body();
fclose(patchfile);
 
-   handle_info();
+   handle_info(mi);
 
return 0;
 }
@@ -1014,17 +1018,26 @@ static int git_mailinfo_config(const char *var, const 
char *value, void *unused)

[PATCH v2 10/31] mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo

[Junio C Hamano]

These two are the only easy ones that do not require passing the
structure around to deep corners of the callchain.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 9e41a10..855d813 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -9,13 +9,13 @@
 
 static FILE *cmitmsg, *patchfile, *fin, *fout;
 
-static int keep_subject;
-static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
 
 struct mailinfo {
struct strbuf name;
struct strbuf email;
+   int keep_subject;
+   int keep_non_patch_brackets_in_subject;
 };
 static char *message_id;
 
@@ -224,7 +224,7 @@ static int is_multipart_boundary(const struct strbuf *line)
!memcmp(line->buf, (*content_top)->buf, (*content_top)->len));
 }
 
-static void cleanup_subject(struct strbuf *subject)
+static void cleanup_subject(struct mailinfo *mi, struct strbuf *subject)
 {
size_t at = 0;
 
@@ -252,7 +252,7 @@ static void cleanup_subject(struct strbuf *subject)
if (!pos)
break;
remove = pos - subject->buf + at + 1;
-   if (!keep_non_patch_brackets_in_subject ||
+   if (!mi->keep_non_patch_brackets_in_subject ||
(7 <= remove &&
 memmem(subject->buf + at, remove, "PATCH", 5)))
strbuf_remove(subject, at, remove);
@@ -947,8 +947,8 @@ static void handle_info(struct mailinfo *mi)
continue;
 
if (!strcmp(header[i], "Subject")) {
-   if (!keep_subject) {
-   cleanup_subject(hdr);
+   if (!mi->keep_subject) {
+   cleanup_subject(mi, hdr);
cleanup_space(hdr);
}
output_header_lines(fout, "Subject", hdr);
@@ -1044,9 +1044,9 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
 
while (1 < argc && argv[1][0] == '-') {
if (!strcmp(argv[1], "-k"))
-   keep_subject = 1;
+   mi.keep_subject = 1;
else if (!strcmp(argv[1], "-b"))
-   keep_non_patch_brackets_in_subject = 1;
+   mi.keep_non_patch_brackets_in_subject = 1;
else if (!strcmp(argv[1], "-m") || !strcmp(argv[1], 
"--message-id"))
add_message_id = 1;
else if (!strcmp(argv[1], "-u"))
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 22/31] mailinfo: keep the parsed log message in a strbuf

[Junio C Hamano]

When mailinfo() is eventually libified, the calling "git am" still
will have to write out the log message in the "msg" file for hooks
and other users of the information, but at least it does not have to
reopen and reread what was written back if the function kept it in a
strbuf so that the caller can read it from there.

This also removes the need for seeking and truncating the output
file when we see a scissors mark in the input, which in turn reduces
two callsites of die_errno().

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 40 +++-
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index aa17c77..72668c9 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -10,7 +10,6 @@
 struct mailinfo {
FILE *input;
FILE *output;
-   FILE *cmitmsg;
FILE *patchfile;
 
struct strbuf name;
@@ -32,6 +31,8 @@ struct mailinfo {
int header_stage; /* still checking in-body headers? */
struct strbuf **p_hdr_data;
struct strbuf **s_hdr_data;
+
+   struct strbuf log_message;
 };
 
 #define MAX_HDR_PARSED 10
@@ -746,10 +747,8 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
-   if (fseek(mi->cmitmsg, 0L, SEEK_SET))
-   die_errno("Could not rewind output message file");
-   if (ftruncate(fileno(mi->cmitmsg), 0))
-   die_errno("Could not truncate output message file at 
scissors");
+
+   strbuf_setlen(>log_message, 0);
mi->header_stage = 1;
 
/*
@@ -766,13 +765,12 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (patchbreak(line)) {
if (mi->message_id)
-   fprintf(mi->cmitmsg, "Message-Id: %s\n", 
mi->message_id);
-   fclose(mi->cmitmsg);
-   mi->cmitmsg = NULL;
+   strbuf_addf(>log_message,
+   "Message-Id: %s\n", mi->message_id);
return 1;
}
 
-   fputs(line->buf, mi->cmitmsg);
+   strbuf_addbuf(>log_message, line);
return 0;
 }
 
@@ -970,18 +968,19 @@ static void handle_info(struct mailinfo *mi)
 
 static int mailinfo(struct mailinfo *mi, const char *msg, const char *patch)
 {
+   FILE *cmitmsg;
int peek;
struct strbuf line = STRBUF_INIT;
 
-   mi->cmitmsg = fopen(msg, "w");
-   if (!mi->cmitmsg) {
+   cmitmsg = fopen(msg, "w");
+   if (!cmitmsg) {
perror(msg);
return -1;
}
mi->patchfile = fopen(patch, "w");
if (!mi->patchfile) {
perror(patch);
-   fclose(mi->cmitmsg);
+   fclose(cmitmsg);
return -1;
}
 
@@ -998,6 +997,8 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
check_header(mi, , mi->p_hdr_data, 1);
 
handle_body(mi, );
+   fwrite(mi->log_message.buf, 1, mi->log_message.len, cmitmsg);
+   fclose(cmitmsg);
fclose(mi->patchfile);
 
handle_info(mi);
@@ -1025,11 +1026,20 @@ static void setup_mailinfo(struct mailinfo *mi)
strbuf_init(>name, 0);
strbuf_init(>email, 0);
strbuf_init(>charset, 0);
+   strbuf_init(>log_message, 0);
mi->header_stage = 1;
mi->use_inbody_headers = 1;
git_config(git_mailinfo_config, );
 }
 
+static void clear_mailinfo(struct mailinfo *mi)
+{
+   strbuf_release(>name);
+   strbuf_release(>email);
+   strbuf_release(>charset);
+   strbuf_release(>log_message);
+}
+
 static const char mailinfo_usage[] =
"git mailinfo [-k | -b] [-m | --message-id] [-u | --encoding= 
| -n] [--scissors | --no-scissors]   < mail >info";
 
@@ -1037,6 +1047,7 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
 {
const char *def_charset;
struct mailinfo mi;
+   int status;
 
/* NEEDSWORK: might want to do the optional .git/ directory
 * discovery
@@ -1075,5 +1086,8 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
 
mi.input = stdin;
mi.output = stdout;
-   return !!mailinfo(, argv[1], argv[2]);
+   status = !!mailinfo(, argv[1], argv[2]);
+   clear_mailinfo();
+
+   return status;
 }
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 30/31] mailinfo: remove calls to exit() and die() deep in the callchain

[Junio C Hamano]

The top-level mailinfo() would instead punt when the code in the
deeper part of the callchain detects an unrecoverable error in the
input.

Signed-off-by: Junio C Hamano 
---
 mailinfo.c | 30 ++
 mailinfo.h |  1 +
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/mailinfo.c b/mailinfo.c
index 7969bf4..cb39c6d 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -163,8 +163,10 @@ static void handle_content_type(struct mailinfo *mi, 
struct strbuf *line)
if (slurp_attr(line->buf, "boundary=", boundary)) {
strbuf_insert(boundary, 0, "--", 2);
if (++mi->content_top >= >content[MAX_BOUNDARIES]) {
-   fprintf(stderr, "Too many boundaries to handle\n");
-   exit(1);
+   error("Too many boundaries to handle");
+   mi->input_error = -1;
+   mi->content_top = >content[MAX_BOUNDARIES] - 1;
+   return;
}
*(mi->content_top) = boundary;
boundary = NULL;
@@ -355,9 +357,11 @@ static int convert_to_utf8(struct mailinfo *mi,
if (same_encoding(mi->metainfo_charset, charset))
return 0;
out = reencode_string(line->buf, mi->metainfo_charset, charset);
-   if (!out)
+   if (!out) {
+   mi->input_error = -1;
return error("cannot convert from %s to %s",
 charset, mi->metainfo_charset);
+   }
strbuf_attach(line, out, strlen(out), strlen(out));
return 0;
 }
@@ -367,6 +371,7 @@ static void decode_header(struct mailinfo *mi, struct 
strbuf *it)
char *in, *ep, *cp;
struct strbuf outbuf = STRBUF_INIT, *dec;
struct strbuf charset_q = STRBUF_INIT, piecebuf = STRBUF_INIT;
+   int found_error = 1; /* pessimism */
 
in = it->buf;
while (in - it->buf <= it->len && (ep = strstr(in, "=?")) != NULL) {
@@ -436,10 +441,14 @@ static void decode_header(struct mailinfo *mi, struct 
strbuf *it)
strbuf_addstr(, in);
strbuf_reset(it);
strbuf_addbuf(it, );
+   found_error = 0;
 release_return:
strbuf_release();
strbuf_release(_q);
strbuf_release();
+
+   if (found_error)
+   mi->input_error = -1;
 }
 
 static int check_header(struct mailinfo *mi,
@@ -640,7 +649,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
/* normalize the log message to UTF-8. */
if (convert_to_utf8(mi, line, mi->charset.buf))
-   exit(128);
+   return 0; /* mi->input_error already set */
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
@@ -783,12 +792,15 @@ again:
   will fail first.  But just in case..
 */
if (--mi->content_top < mi->content) {
-   fprintf(stderr, "Detected mismatched boundaries, "
-   "can't recover\n");
-   exit(1);
+   error("Detected mismatched boundaries, can't recover");
+   mi->input_error = -1;
+   mi->content_top = mi->content;
+   return 0;
}
handle_filter(mi, );
strbuf_release();
+   if (mi->input_error)
+   return 0;
 
/* skip to the next boundary */
if (!find_boundary(mi, line))
@@ -873,6 +885,8 @@ static void handle_body(struct mailinfo *mi, struct strbuf 
*line)
handle_filter(mi, line);
}
 
+   if (mi->input_error)
+   break;
} while (!strbuf_getwholeline(line, mi->input, '\n'));
 
 handle_body_out:
@@ -966,7 +980,7 @@ int mailinfo(struct mailinfo *mi, const char *msg, const 
char *patch)
 
handle_info(mi);
 
-   return 0;
+   return mi->input_error;
 }
 
 static int git_mailinfo_config(const char *var, const char *value, void *mi_)
diff --git a/mailinfo.h b/mailinfo.h
index 27841be..5bf257d 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -31,6 +31,7 @@ struct mailinfo {
struct strbuf **s_hdr_data;
 
struct strbuf log_message;
+   int input_error;
 };
 
 extern void setup_mailinfo(struct mailinfo *);
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 08/31] mailinfo: move global "line" into mailinfo() function

[Junio C Hamano]

With the pregvious step, it becomes clear that the mailinfo()
function is the only one that wants the "line_global" to be directly
touchable.

Logically, this strbuf belongs to handle_body().  It passes the line
to its helper functions for processing, and keeps one line at a time
into the variable in the loop to drive that process.  It would have
been even nicer looking if we could make the variable local to the
function.  But the function has to be passed its initial value
(i.e. the first line is read by its caller), and that is why its
sole caller owns it in the resulting code structure.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 721d999..c8dc73f 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -12,7 +12,6 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
 static int keep_subject;
 static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
-static struct strbuf line_global = STRBUF_INIT;
 static struct strbuf name = STRBUF_INIT;
 static struct strbuf email = STRBUF_INIT;
 static char *message_id;
@@ -966,6 +965,8 @@ static void handle_info(void)
 static int mailinfo(FILE *in, FILE *out, const char *msg, const char *patch)
 {
int peek;
+   struct strbuf line = STRBUF_INIT;
+
fin = in;
fout = out;
 
@@ -990,10 +991,10 @@ static int mailinfo(FILE *in, FILE *out, const char *msg, 
const char *patch)
ungetc(peek, in);
 
/* process the email header */
-   while (read_one_header_line(_global, fin))
-   check_header(_global, p_hdr_data, 1);
+   while (read_one_header_line(, fin))
+   check_header(, p_hdr_data, 1);
 
-   handle_body(_global);
+   handle_body();
fclose(patchfile);
 
handle_info();
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 25/31] mailinfo: move check_header() after the helpers it uses

[Junio C Hamano]

This way, we can lose a forward decl for decode_header().

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 139 ++---
 1 file changed, 69 insertions(+), 70 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 0b083d0..ee669b9 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -293,7 +293,6 @@ static void cleanup_space(struct strbuf *sb)
}
 }
 
-static void decode_header(struct mailinfo *mi, struct strbuf *line);
 static const char *header[MAX_HDR_PARSED] = {
"From","Subject","Date",
 };
@@ -321,75 +320,6 @@ static int is_format_patch_separator(const char *line, int 
len)
return !memcmp(SAMPLE + (cp - line), cp, strlen(SAMPLE) - (cp - line));
 }
 
-static int check_header(struct mailinfo *mi,
-   const struct strbuf *line,
-   struct strbuf *hdr_data[], int overwrite)
-{
-   int i, ret = 0, len;
-   struct strbuf sb = STRBUF_INIT;
-
-   /* search for the interesting parts */
-   for (i = 0; header[i]; i++) {
-   int len = strlen(header[i]);
-   if ((!hdr_data[i] || overwrite) && cmp_header(line, header[i])) 
{
-   /* Unwrap inline B and Q encoding, and optionally
-* normalize the meta information to utf8.
-*/
-   strbuf_add(, line->buf + len + 2, line->len - len - 
2);
-   decode_header(mi, );
-   handle_header(_data[i], );
-   ret = 1;
-   goto check_header_out;
-   }
-   }
-
-   /* Content stuff */
-   if (cmp_header(line, "Content-Type")) {
-   len = strlen("Content-Type: ");
-   strbuf_add(, line->buf + len, line->len - len);
-   decode_header(mi, );
-   strbuf_insert(, 0, "Content-Type: ", len);
-   handle_content_type(mi, );
-   ret = 1;
-   goto check_header_out;
-   }
-   if (cmp_header(line, "Content-Transfer-Encoding")) {
-   len = strlen("Content-Transfer-Encoding: ");
-   strbuf_add(, line->buf + len, line->len - len);
-   decode_header(mi, );
-   handle_content_transfer_encoding(mi, );
-   ret = 1;
-   goto check_header_out;
-   }
-   if (cmp_header(line, "Message-Id")) {
-   len = strlen("Message-Id: ");
-   strbuf_add(, line->buf + len, line->len - len);
-   decode_header(mi, );
-   handle_message_id(mi, );
-   ret = 1;
-   goto check_header_out;
-   }
-
-   /* for inbody stuff */
-   if (starts_with(line->buf, ">From") && isspace(line->buf[5])) {
-   ret = is_format_patch_separator(line->buf + 1, line->len - 1);
-   goto check_header_out;
-   }
-   if (starts_with(line->buf, "[PATCH]") && isspace(line->buf[7])) {
-   for (i = 0; header[i]; i++) {
-   if (!strcmp("Subject", header[i])) {
-   handle_header(_data[i], line);
-   ret = 1;
-   goto check_header_out;
-   }
-   }
-   }
-
-check_header_out:
-   strbuf_release();
-   return ret;
-}
-
 static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
 {
const char *in = q_seg->buf;
@@ -550,6 +480,75 @@ release_return:
strbuf_release();
 }
 
+static int check_header(struct mailinfo *mi,
+   const struct strbuf *line,
+   struct strbuf *hdr_data[], int overwrite)
+{
+   int i, ret = 0, len;
+   struct strbuf sb = STRBUF_INIT;
+
+   /* search for the interesting parts */
+   for (i = 0; header[i]; i++) {
+   int len = strlen(header[i]);
+   if ((!hdr_data[i] || overwrite) && cmp_header(line, header[i])) 
{
+   /* Unwrap inline B and Q encoding, and optionally
+* normalize the meta information to utf8.
+*/
+   strbuf_add(, line->buf + len + 2, line->len - len - 
2);
+   decode_header(mi, );
+   handle_header(_data[i], );
+   ret = 1;
+   goto check_header_out;
+   }
+   }
+
+   /* Content stuff */
+   if (cmp_header(line, "Content-Type")) {
+   len = strlen("Content-Type: ");
+   strbuf_add(, line->buf + len, line->len - len);
+   decode_header(mi, );
+   strbuf_insert(, 0, "Content-Type: ", len);
+   handle_content_type(mi, );
+   ret = 1;
+   goto check_header_out;
+   }
+   if (cmp_header(line, 

[PATCH v2 31/31] am: make direct call to mailinfo

[Junio C Hamano]

And finally the endgame.  Instead of spawning "git mailinfo" via the
run_command() API the same number of times as there are incoming
patches, make direct internal call to the libified mailinfo() from
"git am" to reduce the spawning overhead, which would matter on some
platforms.

Signed-off-by: Junio C Hamano 
---
 builtin/am.c | 42 +++---
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 4f77e07..1873307 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -27,6 +27,7 @@
 #include "notes-utils.h"
 #include "rerere.h"
 #include "prompt.h"
+#include "mailinfo.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -1258,58 +1259,61 @@ static void am_append_signoff(struct am_state *state)
 static int parse_mail(struct am_state *state, const char *mail)
 {
FILE *fp;
-   struct child_process cp = CHILD_PROCESS_INIT;
struct strbuf sb = STRBUF_INIT;
struct strbuf msg = STRBUF_INIT;
struct strbuf author_name = STRBUF_INIT;
struct strbuf author_date = STRBUF_INIT;
struct strbuf author_email = STRBUF_INIT;
int ret = 0;
+   struct mailinfo mi;
 
-   cp.git_cmd = 1;
-   cp.in = xopen(mail, O_RDONLY, 0);
-   cp.out = xopen(am_path(state, "info"), O_WRONLY | O_CREAT, 0777);
+   setup_mailinfo();
 
-   argv_array_push(, "mailinfo");
-   argv_array_push(, state->utf8 ? "-u" : "-n");
+   if (state->utf8)
+   mi.metainfo_charset = get_commit_output_encoding();
+   else
+   mi.metainfo_charset = NULL;
 
switch (state->keep) {
case KEEP_FALSE:
break;
case KEEP_TRUE:
-   argv_array_push(, "-k");
+   mi.keep_subject = 1;
break;
case KEEP_NON_PATCH:
-   argv_array_push(, "-b");
+   mi.keep_non_patch_brackets_in_subject = 1;
break;
default:
die("BUG: invalid value for state->keep");
}
 
if (state->message_id)
-   argv_array_push(, "-m");
+   mi.add_message_id = 1;
 
switch (state->scissors) {
case SCISSORS_UNSET:
break;
case SCISSORS_FALSE:
-   argv_array_push(, "--no-scissors");
+   mi.use_scissors = 0;
break;
case SCISSORS_TRUE:
-   argv_array_push(, "--scissors");
+   mi.use_scissors = 1;
break;
default:
die("BUG: invalid value for state->scissors");
}
 
-   argv_array_push(, am_path(state, "msg"));
-   argv_array_push(, am_path(state, "patch"));
-
-   if (run_command() < 0)
+   mi.input = fopen(mail, "r");
+   if (!mi.input)
+   die("could not open input");
+   mi.output = fopen(am_path(state, "info"), "w");
+   if (!mi.output)
+   die("could not open output 'info'");
+   if (mailinfo(, am_path(state, "msg"), am_path(state, "patch")))
die("could not parse patch");
 
-   close(cp.in);
-   close(cp.out);
+   fclose(mi.input);
+   fclose(mi.output);
 
/* Extract message and author information */
fp = xfopen(am_path(state, "info"), "r");
@@ -1341,8 +1345,7 @@ static int parse_mail(struct am_state *state, const char 
*mail)
}
 
strbuf_addstr(, "\n\n");
-   if (strbuf_read_file(, am_path(state, "msg"), 0) < 0)
-   die_errno(_("could not read '%s'"), am_path(state, "msg"));
+   strbuf_addbuf(, _message);
stripspace(, 0);
 
if (state->signoff)
@@ -1366,6 +1369,7 @@ finish:
strbuf_release(_email);
strbuf_release(_name);
strbuf_release();
+   clear_mailinfo();
return ret;
 }
 
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 14/31] mailinfo: move add_message_id and message_id to struct mailinfo

[Junio C Hamano]

This requires us to pass the structure into check_header() codepath.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 30 --
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index f451ba4..6703453 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -19,12 +19,13 @@ struct mailinfo {
struct strbuf email;
int keep_subject;
int keep_non_patch_brackets_in_subject;
+   int add_message_id;
 
+   char *message_id;
int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
 };
-static char *message_id;
 
 static enum  {
TE_DONTCARE, TE_QP, TE_BASE64
@@ -34,7 +35,6 @@ static struct strbuf charset = STRBUF_INIT;
 
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
-static int add_message_id;
 static int use_inbody_headers = 1;
 
 #define MAX_HDR_PARSED 10
@@ -209,10 +209,10 @@ static void handle_content_type(struct strbuf *line)
}
 }
 
-static void handle_message_id(const struct strbuf *line)
+static void handle_message_id(struct mailinfo *mi, const struct strbuf *line)
 {
-   if (add_message_id)
-   message_id = strdup(line->buf);
+   if (mi->add_message_id)
+   mi->message_id = strdup(line->buf);
 }
 
 static void handle_content_transfer_encoding(const struct strbuf *line)
@@ -321,11 +321,13 @@ static int is_format_patch_separator(const char *line, 
int len)
return !memcmp(SAMPLE + (cp - line), cp, strlen(SAMPLE) - (cp - line));
 }
 
-static int check_header(const struct strbuf *line,
-   struct strbuf *hdr_data[], int overwrite)
+static int check_header(struct mailinfo *mi,
+   const struct strbuf *line,
+   struct strbuf *hdr_data[], int overwrite)
 {
int i, ret = 0, len;
struct strbuf sb = STRBUF_INIT;
+
/* search for the interesting parts */
for (i = 0; header[i]; i++) {
int len = strlen(header[i]);
@@ -363,7 +365,7 @@ static int check_header(const struct strbuf *line,
len = strlen("Message-Id: ");
strbuf_add(, line->buf + len, line->len - len);
decode_header();
-   handle_message_id();
+   handle_message_id(mi, );
ret = 1;
goto check_header_out;
}
@@ -733,7 +735,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
}
 
if (use_inbody_headers && mi->header_stage) {
-   mi->header_stage = check_header(line, s_hdr_data, 0);
+   mi->header_stage = check_header(mi, line, s_hdr_data, 0);
if (mi->header_stage)
return 0;
} else
@@ -767,8 +769,8 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
}
 
if (patchbreak(line)) {
-   if (message_id)
-   fprintf(cmitmsg, "Message-Id: %s\n", message_id);
+   if (mi->message_id)
+   fprintf(cmitmsg, "Message-Id: %s\n", mi->message_id);
fclose(cmitmsg);
cmitmsg = NULL;
return 1;
@@ -843,7 +845,7 @@ again:
 
/* slurp in this section's info */
while (read_one_header_line(line, mi->input))
-   check_header(line, p_hdr_data, 0);
+   check_header(mi, line, p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
@@ -997,7 +999,7 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
 
/* process the email header */
while (read_one_header_line(, mi->input))
-   check_header(, p_hdr_data, 1);
+   check_header(mi, , p_hdr_data, 1);
 
handle_body(mi, );
fclose(patchfile);
@@ -1050,7 +1052,7 @@ int cmd_mailinfo(int argc, const char **argv, const char 
*prefix)
else if (!strcmp(argv[1], "-b"))
mi.keep_non_patch_brackets_in_subject = 1;
else if (!strcmp(argv[1], "-m") || !strcmp(argv[1], 
"--message-id"))
-   add_message_id = 1;
+   mi.add_message_id = 1;
else if (!strcmp(argv[1], "-u"))
metainfo_charset = def_charset;
else if (!strcmp(argv[1], "-n"))
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 29/31] mailinfo: handle charset conversion errors in the caller

[Junio C Hamano]

Instead of dying in convert_to_utf8(), just report an error and let
the callers handle it.  Between the two callers:

 - decode_header() silently punts when it cannot parse a broken
   RFC2047 encoded text (e.g. when it sees anything other than B or
   Q after it sees "=?") by jumping to release_return,
   returning the string it successfully parsed out so far, to the
   caller.  A piece of string that convert_to_utf8() cannot handle
   can be treated the same way.

 - handle_commit_msg() doesn't cope with a malformed line well, so
   die there for now.  We'll lift this even higher in later changes
   in this series.

Signed-off-by: Junio C Hamano 
---
 mailinfo.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/mailinfo.c b/mailinfo.c
index 59479f6..7969bf4 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -344,21 +344,22 @@ static struct strbuf *decode_b_segment(const struct 
strbuf *b_seg)
return out;
 }
 
-static void convert_to_utf8(struct mailinfo *mi,
-   struct strbuf *line, const char *charset)
+static int convert_to_utf8(struct mailinfo *mi,
+  struct strbuf *line, const char *charset)
 {
char *out;
 
if (!mi->metainfo_charset || !charset || !*charset)
-   return;
+   return 0;
 
if (same_encoding(mi->metainfo_charset, charset))
-   return;
+   return 0;
out = reencode_string(line->buf, mi->metainfo_charset, charset);
if (!out)
-   die("cannot convert from %s to %s",
-   charset, mi->metainfo_charset);
+   return error("cannot convert from %s to %s",
+charset, mi->metainfo_charset);
strbuf_attach(line, out, strlen(out), strlen(out));
+   return 0;
 }
 
 static void decode_header(struct mailinfo *mi, struct strbuf *it)
@@ -424,7 +425,8 @@ static void decode_header(struct mailinfo *mi, struct 
strbuf *it)
dec = decode_q_segment(, 1);
break;
}
-   convert_to_utf8(mi, dec, charset_q.buf);
+   if (convert_to_utf8(mi, dec, charset_q.buf))
+   goto release_return;
 
strbuf_addbuf(, dec);
strbuf_release(dec);
@@ -637,7 +639,8 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
-   convert_to_utf8(mi, line, mi->charset.buf);
+   if (convert_to_utf8(mi, line, mi->charset.buf))
+   exit(128);
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 27/31] mailinfo: move definition of MAX_HDR_PARSED to closer to its use

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index f4771ee..de446ec 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -39,8 +39,6 @@ struct mailinfo {
struct strbuf log_message;
 };
 
-#define MAX_HDR_PARSED 10
-
 static void cleanup_space(struct strbuf *sb)
 {
size_t pos, cnt;
@@ -290,6 +288,7 @@ static void cleanup_subject(struct mailinfo *mi, struct 
strbuf *subject)
strbuf_trim(subject);
 }
 
+#define MAX_HDR_PARSED 10
 static const char *header[MAX_HDR_PARSED] = {
"From","Subject","Date",
 };
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 13/31] mailinfo: move patch_lines to struct mailinfo

[Junio C Hamano]

This one is trivial thanks to previous steps that started passing
the structure throughout the input codepaths.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 5d2d88a..f451ba4 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -20,6 +20,7 @@ struct mailinfo {
int keep_subject;
int keep_non_patch_brackets_in_subject;
 
+   int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
 };
@@ -30,7 +31,6 @@ static enum  {
 } transfer_encoding;
 
 static struct strbuf charset = STRBUF_INIT;
-static int patch_lines;
 
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
@@ -778,10 +778,10 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
return 0;
 }
 
-static void handle_patch(const struct strbuf *line)
+static void handle_patch(struct mailinfo *mi, const struct strbuf *line)
 {
fwrite(line->buf, 1, line->len, patchfile);
-   patch_lines++;
+   mi->patch_lines++;
 }
 
 static void handle_filter(struct mailinfo *mi, struct strbuf *line)
@@ -792,7 +792,7 @@ static void handle_filter(struct mailinfo *mi, struct 
strbuf *line)
break;
mi->filter_stage++;
case 1:
-   handle_patch(line);
+   handle_patch(mi, line);
break;
}
 }
@@ -944,7 +944,7 @@ static void handle_info(struct mailinfo *mi)
 
for (i = 0; header[i]; i++) {
/* only print inbody headers if we output a patch file */
-   if (patch_lines && s_hdr_data[i])
+   if (mi->patch_lines && s_hdr_data[i])
hdr = s_hdr_data[i];
else if (p_hdr_data[i])
hdr = p_hdr_data[i];
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 03/31] mailinfo: explicitly close file handle to the patch output

[Junio C Hamano]

This does not make a difference within the context of "git mailinfo"
that runs once and exits, as flushing and closing would happen upon
process completion.  It however will matter when we eventually make
this function callable as an API function.

Besides, cleaning after yourself once you are done is a good hygiene.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index d3756cc..412a23b 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -1008,6 +1008,8 @@ static int mailinfo(FILE *in, FILE *out, const char *msg, 
const char *patch)
check_header(, p_hdr_data, 1);
 
handle_body();
+   fclose(patchfile);
+
handle_info();
 
return 0;
-- 
2.6.1-320-g86a1181

--
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: repository corruption when pushing commits to a repository running 'git gc --prune='

[Matthieu Moy]

Junio C Hamano  writes:

> Matthieu Moy  writes:
>
>> This looks like a race between deleting objects/05/ and creating
>> objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed.
>>
>> I don't understand how this is possible, since the temporary file and
>> the final one are in the same directory, so deleting the directory
>> should fail.
>>
>> What am I missing?
>
>  1. tmp_obj_* gets created in objects/05/ by "git unpack-objects";
>
>  2. prune_cruft() comes, notices tmp_obj_* that is sufficiently old,
> decides to remove in "git prune".

Ah, OK, the real problem is the temporary file removal. Thanks.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 06/26] mailinfo: always pass "line" as an argument

[Stefan Beller]

On Tue, Oct 13, 2015 at 4:16 PM, Junio C Hamano  wrote:
> Some functions in this module accessed the global "struct strbuf
> line" while many others used a strbuf passed as an argument.
> Convert the former to ensure that nobody deeper in the callchains
> relies on the global one.
>
> Signed-off-by: Junio C Hamano 
> ---
>  builtin/mailinfo.c | 48 
>  1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
> index 5a74811..c3c7d67 100644
> --- a/builtin/mailinfo.c
> +++ b/builtin/mailinfo.c
> @@ -12,7 +12,7 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
>  static int keep_subject;
>  static int keep_non_patch_brackets_in_subject;
>  static const char *metainfo_charset;
> -static struct strbuf line = STRBUF_INIT;
> +static struct strbuf line_global = STRBUF_INIT;
>  static struct strbuf name = STRBUF_INIT;
>  static struct strbuf email = STRBUF_INIT;
>  static char *message_id;
> @@ -817,23 +817,23 @@ static void handle_filter(struct strbuf *line, int 
> *filter_stage, int *header_st
> }
>  }
>
> -static int find_boundary(void)
> +static int find_boundary(struct strbuf *line)
>  {
> -   while (!strbuf_getline(, fin, '\n')) {
> -   if (*content_top && is_multipart_boundary())
> +   while (!strbuf_getline(line, fin, '\n')) {
> +   if (*content_top && is_multipart_boundary(line))
> return 1;
> }
> return 0;
>  }
>
> -static int handle_boundary(int *filter_stage, int *header_stage)
> +static int handle_boundary(struct strbuf *line, int *filter_stage, int 
> *header_stage)
>  {
> struct strbuf newline = STRBUF_INIT;
>
> strbuf_addch(, '\n');
>  again:
> -   if (line.len >= (*content_top)->len + 2 &&
> -   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
> +   if (line->len >= (*content_top)->len + 2 &&
> +   !memcmp(line->buf + (*content_top)->len, "--", 2)) {
> /* we hit an end boundary */
> /* pop the current boundary off the stack */
> strbuf_release(*content_top);
> @@ -852,7 +852,7 @@ again:
> strbuf_release();
>
> /* skip to the next boundary */
> -   if (!find_boundary())
> +   if (!find_boundary(line))
> return 0;
> goto again;
> }
> @@ -862,18 +862,18 @@ again:
> strbuf_reset();
>
> /* slurp in this section's info */
> -   while (read_one_header_line(, fin))
> -   check_header(, p_hdr_data, 0);
> +   while (read_one_header_line(line, fin))
> +   check_header(line, p_hdr_data, 0);
>
> strbuf_release();
> /* replenish line */
> -   if (strbuf_getline(, fin, '\n'))
> +   if (strbuf_getline(line, fin, '\n'))
> return 0;
> -   strbuf_addch(, '\n');
> +   strbuf_addch(line, '\n');
> return 1;
>  }
>
> -static void handle_body(void)
> +static void handle_body(struct strbuf *line)
>  {
> struct strbuf prev = STRBUF_INIT;
> int filter_stage = 0;
> @@ -881,24 +881,24 @@ static void handle_body(void)
>
> /* Skip up to the first boundary */
> if (*content_top) {
> -   if (!find_boundary())
> +   if (!find_boundary(line))
> goto handle_body_out;
> }
>
> do {
> /* process any boundary lines */
> -   if (*content_top && is_multipart_boundary()) {
> +   if (*content_top && is_multipart_boundary(line)) {
> /* flush any leftover */
> if (prev.len) {
> handle_filter(, _stage, 
> _stage);
> strbuf_reset();
> }
> -   if (!handle_boundary(_stage, _stage))
> +   if (!handle_boundary(line, _stage, 
> _stage))
> goto handle_body_out;
> }
>
> /* Unwrap transfer encoding */
> -   decode_transfer_encoding();
> +   decode_transfer_encoding(line);
>
> switch (transfer_encoding) {
> case TE_BASE64:
> @@ -907,7 +907,7 @@ static void handle_body(void)
> struct strbuf **lines, **it, *sb;
>
> /* Prepend any previous partial lines */
> -   strbuf_insert(, 0, prev.buf, prev.len);
> +   strbuf_insert(line, 0, prev.buf, prev.len);
> strbuf_reset();
>
> /*
> @@ -915,7 +915,7 @@ static void handle_body(void)
>  * multiple new lines.  Pass only one chunk
>  * at a time to handle_filter()
>  */
> -   

Re: [PATCH 07/26] mailinfo: move global "line" into mailinfo() function

[Stefan Beller]

On Tue, Oct 13, 2015 at 4:16 PM, Junio C Hamano  wrote:
> The mailinfo() function is the only one that wants the "line_global"
> to be directly touchable.  Note that handle_body() has to be passed
> this strbuf so that it sees the "first line of the input" after the
> loop in this function processes the headers.  It feels a bit dirty
> that handle_body() then keeps reusing this strbuf to read more lines
> and does its processing, but that is how the code is structured now.
>
> Signed-off-by: Junio C Hamano 

Ok, that is exactly what I thought about squashing into patch 6.
A separate patch will do too.
--
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 06/26] mailinfo: always pass "line" as an argument

[Junio C Hamano]

Stefan Beller  writes:

>> @@ -1019,10 +1019,10 @@ static int mailinfo(FILE *in, FILE *out, const char 
>> *msg, const char *patch)
>> ungetc(peek, in);
>>
>> /* process the email header */
>> -   while (read_one_header_line(, fin))
>> -   check_header(, p_hdr_data, 1);
>> +   while (read_one_header_line(_global, fin))
>> +   check_header(_global, p_hdr_data, 1);
>
> This is the only function to use line_global if I see correctly.
> The function is called only once, so no need to preserve state
> outside the function. Would it make sense to remove line_global
> completely and have a local variable in this function instead?

That is exactly the step that comes after it does, but if you squash
06 and 07 into one patch (i.e. take diff between the state after 05
and after 07), that realization will not easily come (well, at least
it didn't come to me and I wasn't convinced that the conversion is
correct myself until I split 06 and 07 into two separate steps).


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 04/31] mailinfo: fold decode_header_bq() into decode_header()

[Junio C Hamano]

In olden days we might have wanted to behave differently in
decode_header() if the header line was encoded with RFC2047, but we
apparently do not do so, hence this helper function can go, together
with its return value.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 23 +++
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 412a23b..73be47c 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -525,19 +525,17 @@ static void convert_to_utf8(struct strbuf *line, const 
char *charset)
strbuf_attach(line, out, strlen(out), strlen(out));
 }
 
-static int decode_header_bq(struct strbuf *it)
+static void decode_header(struct strbuf *it)
 {
char *in, *ep, *cp;
struct strbuf outbuf = STRBUF_INIT, *dec;
struct strbuf charset_q = STRBUF_INIT, piecebuf = STRBUF_INIT;
-   int rfc2047 = 0;
 
in = it->buf;
while (in - it->buf <= it->len && (ep = strstr(in, "=?")) != NULL) {
int encoding;
strbuf_reset(_q);
strbuf_reset();
-   rfc2047 = 1;
 
if (in != ep) {
/*
@@ -567,22 +565,22 @@ static int decode_header_bq(struct strbuf *it)
ep += 2;
 
if (ep - it->buf >= it->len || !(cp = strchr(ep, '?')))
-   goto decode_header_bq_out;
+   goto release_return;
 
if (cp + 3 - it->buf > it->len)
-   goto decode_header_bq_out;
+   goto release_return;
strbuf_add(_q, ep, cp - ep);
 
encoding = cp[1];
if (!encoding || cp[2] != '?')
-   goto decode_header_bq_out;
+   goto release_return;
ep = strstr(cp + 3, "?=");
if (!ep)
-   goto decode_header_bq_out;
+   goto release_return;
strbuf_add(, cp + 3, ep - cp - 3);
switch (tolower(encoding)) {
default:
-   goto decode_header_bq_out;
+   goto release_return;
case 'b':
dec = decode_b_segment();
break;
@@ -601,17 +599,10 @@ static int decode_header_bq(struct strbuf *it)
strbuf_addstr(, in);
strbuf_reset(it);
strbuf_addbuf(it, );
-decode_header_bq_out:
+release_return:
strbuf_release();
strbuf_release(_q);
strbuf_release();
-   return rfc2047;
-}
-
-static void decode_header(struct strbuf *it)
-{
-   if (decode_header_bq(it))
-   return;
 }
 
 static void decode_transfer_encoding(struct strbuf *line)
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 23/31] mailinfo: move content/content_top to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 45 ++---
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 72668c9..138ca3b 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -7,6 +7,8 @@
 #include "utf8.h"
 #include "strbuf.h"
 
+#define MAX_BOUNDARIES 5
+
 struct mailinfo {
FILE *input;
FILE *output;
@@ -21,6 +23,8 @@ struct mailinfo {
int use_inbody_headers; /* defaults to 1 */
const char *metainfo_charset;
 
+   struct strbuf *content[MAX_BOUNDARIES];
+   struct strbuf **content_top;
struct strbuf charset;
char *message_id;
enum  {
@@ -36,7 +40,6 @@ struct mailinfo {
 };
 
 #define MAX_HDR_PARSED 10
-#define MAX_BOUNDARIES 5
 
 static void cleanup_space(struct strbuf *sb);
 
@@ -181,10 +184,6 @@ static int slurp_attr(const char *line, const char *name, 
struct strbuf *attr)
return 1;
 }
 
-static struct strbuf *content[MAX_BOUNDARIES];
-
-static struct strbuf **content_top = content;
-
 static void handle_content_type(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf *boundary = xmalloc(sizeof(struct strbuf));
@@ -192,11 +191,11 @@ static void handle_content_type(struct mailinfo *mi, 
struct strbuf *line)
 
if (slurp_attr(line->buf, "boundary=", boundary)) {
strbuf_insert(boundary, 0, "--", 2);
-   if (++content_top >= [MAX_BOUNDARIES]) {
+   if (++mi->content_top >= >content[MAX_BOUNDARIES]) {
fprintf(stderr, "Too many boundaries to handle\n");
exit(1);
}
-   *content_top = boundary;
+   *(mi->content_top) = boundary;
boundary = NULL;
}
slurp_attr(line->buf, "charset=", >charset);
@@ -224,10 +223,12 @@ static void handle_content_transfer_encoding(struct 
mailinfo *mi,
mi->transfer_encoding = TE_DONTCARE;
 }
 
-static int is_multipart_boundary(const struct strbuf *line)
+static int is_multipart_boundary(struct mailinfo *mi, const struct strbuf 
*line)
 {
-   return (((*content_top)->len <= line->len) &&
-   !memcmp(line->buf, (*content_top)->buf, (*content_top)->len));
+   struct strbuf *content_top = *(mi->content_top);
+
+   return ((content_top->len <= line->len) &&
+   !memcmp(line->buf, content_top->buf, content_top->len));
 }
 
 static void cleanup_subject(struct mailinfo *mi, struct strbuf *subject)
@@ -796,7 +797,7 @@ static void handle_filter(struct mailinfo *mi, struct 
strbuf *line)
 static int find_boundary(struct mailinfo *mi, struct strbuf *line)
 {
while (!strbuf_getline(line, mi->input, '\n')) {
-   if (*content_top && is_multipart_boundary(line))
+   if (*(mi->content_top) && is_multipart_boundary(mi, line))
return 1;
}
return 0;
@@ -808,18 +809,18 @@ static int handle_boundary(struct mailinfo *mi, struct 
strbuf *line)
 
strbuf_addch(, '\n');
 again:
-   if (line->len >= (*content_top)->len + 2 &&
-   !memcmp(line->buf + (*content_top)->len, "--", 2)) {
+   if (line->len >= (*(mi->content_top))->len + 2 &&
+   !memcmp(line->buf + (*(mi->content_top))->len, "--", 2)) {
/* we hit an end boundary */
/* pop the current boundary off the stack */
-   strbuf_release(*content_top);
-   free(*content_top);
-   *content_top = NULL;
+   strbuf_release(*(mi->content_top));
+   free(*(mi->content_top));
+   *(mi->content_top) = NULL;
 
/* technically won't happen as is_multipart_boundary()
   will fail first.  But just in case..
 */
-   if (--content_top < content) {
+   if (--mi->content_top < mi->content) {
fprintf(stderr, "Detected mismatched boundaries, "
"can't recover\n");
exit(1);
@@ -854,14 +855,14 @@ static void handle_body(struct mailinfo *mi, struct 
strbuf *line)
struct strbuf prev = STRBUF_INIT;
 
/* Skip up to the first boundary */
-   if (*content_top) {
+   if (*(mi->content_top)) {
if (!find_boundary(mi, line))
goto handle_body_out;
}
 
do {
/* process any boundary lines */
-   if (*content_top && is_multipart_boundary(line)) {
+   if (*(mi->content_top) && is_multipart_boundary(mi, line)) {
/* flush any leftover */
if (prev.len) {
handle_filter(mi, );
@@ -1029,6 +1030,7 @@ static void setup_mailinfo(struct mailinfo *mi)
strbuf_init(>log_message, 0);
mi->header_stage = 

[PATCH v2 07/31] mailinfo: always pass "line" as an argument

[Junio C Hamano]

Some functions in this module accessed the global "struct strbuf
line" while many others used a strbuf passed as an argument.
Convert the former to ensure that nobody deeper in the callchains
relies on the global one.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 48 
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 1518708..721d999 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -12,7 +12,7 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
 static int keep_subject;
 static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
-static struct strbuf line = STRBUF_INIT;
+static struct strbuf line_global = STRBUF_INIT;
 static struct strbuf name = STRBUF_INIT;
 static struct strbuf email = STRBUF_INIT;
 static char *message_id;
@@ -788,23 +788,23 @@ static void handle_filter(struct strbuf *line, int 
*filter_stage, int *header_st
}
 }
 
-static int find_boundary(void)
+static int find_boundary(struct strbuf *line)
 {
-   while (!strbuf_getline(, fin, '\n')) {
-   if (*content_top && is_multipart_boundary())
+   while (!strbuf_getline(line, fin, '\n')) {
+   if (*content_top && is_multipart_boundary(line))
return 1;
}
return 0;
 }
 
-static int handle_boundary(int *filter_stage, int *header_stage)
+static int handle_boundary(struct strbuf *line, int *filter_stage, int 
*header_stage)
 {
struct strbuf newline = STRBUF_INIT;
 
strbuf_addch(, '\n');
 again:
-   if (line.len >= (*content_top)->len + 2 &&
-   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
+   if (line->len >= (*content_top)->len + 2 &&
+   !memcmp(line->buf + (*content_top)->len, "--", 2)) {
/* we hit an end boundary */
/* pop the current boundary off the stack */
strbuf_release(*content_top);
@@ -823,7 +823,7 @@ again:
strbuf_release();
 
/* skip to the next boundary */
-   if (!find_boundary())
+   if (!find_boundary(line))
return 0;
goto again;
}
@@ -833,18 +833,18 @@ again:
strbuf_reset();
 
/* slurp in this section's info */
-   while (read_one_header_line(, fin))
-   check_header(, p_hdr_data, 0);
+   while (read_one_header_line(line, fin))
+   check_header(line, p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
-   if (strbuf_getline(, fin, '\n'))
+   if (strbuf_getline(line, fin, '\n'))
return 0;
-   strbuf_addch(, '\n');
+   strbuf_addch(line, '\n');
return 1;
 }
 
-static void handle_body(void)
+static void handle_body(struct strbuf *line)
 {
struct strbuf prev = STRBUF_INIT;
int filter_stage = 0;
@@ -852,24 +852,24 @@ static void handle_body(void)
 
/* Skip up to the first boundary */
if (*content_top) {
-   if (!find_boundary())
+   if (!find_boundary(line))
goto handle_body_out;
}
 
do {
/* process any boundary lines */
-   if (*content_top && is_multipart_boundary()) {
+   if (*content_top && is_multipart_boundary(line)) {
/* flush any leftover */
if (prev.len) {
handle_filter(, _stage, 
_stage);
strbuf_reset();
}
-   if (!handle_boundary(_stage, _stage))
+   if (!handle_boundary(line, _stage, 
_stage))
goto handle_body_out;
}
 
/* Unwrap transfer encoding */
-   decode_transfer_encoding();
+   decode_transfer_encoding(line);
 
switch (transfer_encoding) {
case TE_BASE64:
@@ -878,7 +878,7 @@ static void handle_body(void)
struct strbuf **lines, **it, *sb;
 
/* Prepend any previous partial lines */
-   strbuf_insert(, 0, prev.buf, prev.len);
+   strbuf_insert(line, 0, prev.buf, prev.len);
strbuf_reset();
 
/*
@@ -886,7 +886,7 @@ static void handle_body(void)
 * multiple new lines.  Pass only one chunk
 * at a time to handle_filter()
 */
-   lines = strbuf_split(, '\n');
+   lines = strbuf_split(line, '\n');
for (it = lines; (sb = *it); it++) {
if (*(it + 1) == NULL) /* The last line */
if (sb->buf[sb->len - 1] != '\n') {

[PATCH v2 00/31] libify mailinfo and call it directly from am

[Junio C Hamano]

During the discussion on the recent "git am" regression, I noticed
that the command reimplemented in C spawns one "mailsplit" and then
spawns "mailinfo" followed by "apply --index" to commit the changes
described in each message.  As there are platforms where spawning
subprocess via run_command() interface is heavy-weight, something
that is conceptually very simple like "mailinfo" is better called
directly inside the process---something that is lightweight and
frequently used is where the overhead of run_command() would be felt
most.

And here is that topic, slightly reordered and polished up, since
yesterday's version.

 - The series no longer depends on an unrelated "how about this" patch
   to mailinfo.

 - Fixes are moved to earlier spots in the series.

 - Error checking in the endgame patch has been tightened.

Junio C Hamano (31):
  mailinfo: remove a no-op call convert_to_utf8(it, "")
  mailinfo: fix for off-by-one error in boundary stack
  mailinfo: explicitly close file handle to the patch output
  mailinfo: fold decode_header_bq() into decode_header()
  mailinfo: move handle_boundary() lower
  mailinfo: get rid of function-local static states
  mailinfo: always pass "line" as an argument
  mailinfo: move global "line" into mailinfo() function
  mailinfo: introduce "struct mailinfo" to hold globals
  mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo
  mailinfo: move global "FILE *fin, *fout" to struct mailinfo
  mailinfo: move filter/header stage to struct mailinfo
  mailinfo: move patch_lines to struct mailinfo
  mailinfo: move add_message_id and message_id to struct mailinfo
  mailinfo: move use_scissors and use_inbody_headers to struct mailinfo
  mailinfo: move metainfo_charset to struct mailinfo
  mailinfo: move transfer_encoding to struct mailinfo
  mailinfo: move charset to struct mailinfo
  mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak
  mailinfo: move cmitmsg and patchfile to struct mailinfo
  mailinfo: move [ps]_hdr_data to struct mailinfo
  mailinfo: keep the parsed log message in a strbuf
  mailinfo: move content/content_top to struct mailinfo
  mailinfo: move read_one_header_line() closer to its callers
  mailinfo: move check_header() after the helpers it uses
  mailinfo: move cleanup_space() before its users
  mailinfo: move definition of MAX_HDR_PARSED to closer to its use
  mailinfo: libify
  mailinfo: handle charset conversion errors in the caller
  mailinfo: remove calls to exit() and die() deep in the callchain
  am: make direct call to mailinfo

 Makefile   |1 +
 builtin/am.c   |   42 ++-
 builtin/mailinfo.c | 1055 +---
 mailinfo.c | 1024 ++
 mailinfo.h |   41 ++
 5 files changed, 1109 insertions(+), 1054 deletions(-)
 create mode 100644 mailinfo.c
 create mode 100644 mailinfo.h

-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 19/31] mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak

[Junio C Hamano]

There is a strange "if (!cmitmsg) return 0" at the very beginning of
handle_commit_msg(), but the condition should never trigger,
because:

 * The only place cmitmsg is set to NULL is after this function sees
   a patch break, closes the FILE * to write the commit log message
   and returns 1.  This function returns non-zero only from that
   codepath.

 * The caller of this function, upon seeing a non-zero return,
   increments filter_stage, starts treating the input as patch text
   and will never call handle_commit_msg() again.

Replace it with an assert(!mi->filter_stage).

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 41f659d..f48a260 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -724,8 +724,7 @@ static int is_scissors_line(const struct strbuf *line)
 
 static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
 {
-   if (!cmitmsg)
-   return 0;
+   assert(!mi->filter_stage);
 
if (mi->header_stage) {
if (!line->len || (line->len == 1 && line->buf[0] == '\n'))
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 20/31] mailinfo: move cmitmsg and patchfile to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index f48a260..5809e13 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -7,11 +7,11 @@
 #include "utf8.h"
 #include "strbuf.h"
 
-static FILE *cmitmsg, *patchfile;
-
 struct mailinfo {
FILE *input;
FILE *output;
+   FILE *cmitmsg;
+   FILE *patchfile;
 
struct strbuf name;
struct strbuf email;
@@ -746,9 +746,9 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
-   if (fseek(cmitmsg, 0L, SEEK_SET))
+   if (fseek(mi->cmitmsg, 0L, SEEK_SET))
die_errno("Could not rewind output message file");
-   if (ftruncate(fileno(cmitmsg), 0))
+   if (ftruncate(fileno(mi->cmitmsg), 0))
die_errno("Could not truncate output message file at 
scissors");
mi->header_stage = 1;
 
@@ -766,19 +766,19 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 
if (patchbreak(line)) {
if (mi->message_id)
-   fprintf(cmitmsg, "Message-Id: %s\n", mi->message_id);
-   fclose(cmitmsg);
-   cmitmsg = NULL;
+   fprintf(mi->cmitmsg, "Message-Id: %s\n", 
mi->message_id);
+   fclose(mi->cmitmsg);
+   mi->cmitmsg = NULL;
return 1;
}
 
-   fputs(line->buf, cmitmsg);
+   fputs(line->buf, mi->cmitmsg);
return 0;
 }
 
 static void handle_patch(struct mailinfo *mi, const struct strbuf *line)
 {
-   fwrite(line->buf, 1, line->len, patchfile);
+   fwrite(line->buf, 1, line->len, mi->patchfile);
mi->patch_lines++;
 }
 
@@ -973,15 +973,15 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
int peek;
struct strbuf line = STRBUF_INIT;
 
-   cmitmsg = fopen(msg, "w");
-   if (!cmitmsg) {
+   mi->cmitmsg = fopen(msg, "w");
+   if (!mi->cmitmsg) {
perror(msg);
return -1;
}
-   patchfile = fopen(patch, "w");
-   if (!patchfile) {
+   mi->patchfile = fopen(patch, "w");
+   if (!mi->patchfile) {
perror(patch);
-   fclose(cmitmsg);
+   fclose(mi->cmitmsg);
return -1;
}
 
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 02/31] mailinfo: fix for off-by-one error in boundary stack

[Junio C Hamano]

We pre-increment the pointer that we will use to store something at,
so the pointer is already beyond the end of the array if it points
at content[MAX_BOUNDARIES].

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 5a4ed75..d3756cc 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -185,7 +185,7 @@ static void handle_content_type(struct strbuf *line)
 
if (slurp_attr(line->buf, "boundary=", boundary)) {
strbuf_insert(boundary, 0, "--", 2);
-   if (++content_top > [MAX_BOUNDARIES]) {
+   if (++content_top >= [MAX_BOUNDARIES]) {
fprintf(stderr, "Too many boundaries to handle\n");
exit(1);
}
-- 
2.6.1-320-g86a1181

--
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: repository corruption when pushing commits to a repository running 'git gc --prune='

[Junio C Hamano]

Matthieu Moy  writes:

> Looks good to me. I think the same should be added to git-prune.txt
> under --expire.

I thought about it, but decided against it, as the command is not
even recommended to end users.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 01/31] mailinfo: remove a no-op call convert_to_utf8(it, "")

[Junio C Hamano]

The called function checks if the second parameter is either a NULL
or an empty string at the very beginning and returns without doing
anything.  Remove the useless call.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 999a525..5a4ed75 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -612,11 +612,6 @@ static void decode_header(struct strbuf *it)
 {
if (decode_header_bq(it))
return;
-   /* otherwise "it" is a straight copy of the input.
-* This can be binary guck but there is no charset specified.
-*/
-   if (metainfo_charset)
-   convert_to_utf8(it, "");
 }
 
 static void decode_transfer_encoding(struct strbuf *line)
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 21/31] mailinfo: move [ps]_hdr_data to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 5809e13..aa17c77 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -30,10 +30,10 @@ struct mailinfo {
int patch_lines;
int filter_stage; /* still reading log or are we copying patch? */
int header_stage; /* still checking in-body headers? */
+   struct strbuf **p_hdr_data;
+   struct strbuf **s_hdr_data;
 };
 
-static struct strbuf **p_hdr_data, **s_hdr_data;
-
 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
 
@@ -732,7 +732,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
}
 
if (mi->use_inbody_headers && mi->header_stage) {
-   mi->header_stage = check_header(mi, line, s_hdr_data, 0);
+   mi->header_stage = check_header(mi, line, mi->s_hdr_data, 0);
if (mi->header_stage)
return 0;
} else
@@ -757,9 +757,9 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
 * them to give ourselves a clean restart.
 */
for (i = 0; header[i]; i++) {
-   if (s_hdr_data[i])
-   strbuf_release(s_hdr_data[i]);
-   s_hdr_data[i] = NULL;
+   if (mi->s_hdr_data[i])
+   strbuf_release(mi->s_hdr_data[i]);
+   mi->s_hdr_data[i] = NULL;
}
return 0;
}
@@ -841,7 +841,7 @@ again:
 
/* slurp in this section's info */
while (read_one_header_line(line, mi->input))
-   check_header(mi, line, p_hdr_data, 0);
+   check_header(mi, line, mi->p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
@@ -942,10 +942,10 @@ static void handle_info(struct mailinfo *mi)
 
for (i = 0; header[i]; i++) {
/* only print inbody headers if we output a patch file */
-   if (mi->patch_lines && s_hdr_data[i])
-   hdr = s_hdr_data[i];
-   else if (p_hdr_data[i])
-   hdr = p_hdr_data[i];
+   if (mi->patch_lines && mi->s_hdr_data[i])
+   hdr = mi->s_hdr_data[i];
+   else if (mi->p_hdr_data[i])
+   hdr = mi->p_hdr_data[i];
else
continue;
 
@@ -985,8 +985,8 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
return -1;
}
 
-   p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*p_hdr_data));
-   s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*s_hdr_data));
+   mi->p_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*(mi->p_hdr_data)));
+   mi->s_hdr_data = xcalloc(MAX_HDR_PARSED, sizeof(*(mi->s_hdr_data)));
 
do {
peek = fgetc(mi->input);
@@ -995,10 +995,10 @@ static int mailinfo(struct mailinfo *mi, const char *msg, 
const char *patch)
 
/* process the email header */
while (read_one_header_line(, mi->input))
-   check_header(mi, , p_hdr_data, 1);
+   check_header(mi, , mi->p_hdr_data, 1);
 
handle_body(mi, );
-   fclose(patchfile);
+   fclose(mi->patchfile);
 
handle_info(mi);
 
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 18/31] mailinfo: move charset to struct mailinfo

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 140b31ee..41f659d 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -22,6 +22,7 @@ struct mailinfo {
int use_inbody_headers; /* defaults to 1 */
const char *metainfo_charset;
 
+   struct strbuf charset;
char *message_id;
enum  {
TE_DONTCARE, TE_QP, TE_BASE64
@@ -31,9 +32,6 @@ struct mailinfo {
int header_stage; /* still checking in-body headers? */
 };
 
-
-static struct strbuf charset = STRBUF_INIT;
-
 static struct strbuf **p_hdr_data, **s_hdr_data;
 
 #define MAX_HDR_PARSED 10
@@ -186,7 +184,7 @@ static struct strbuf *content[MAX_BOUNDARIES];
 
 static struct strbuf **content_top = content;
 
-static void handle_content_type(struct strbuf *line)
+static void handle_content_type(struct mailinfo *mi, struct strbuf *line)
 {
struct strbuf *boundary = xmalloc(sizeof(struct strbuf));
strbuf_init(boundary, line->len);
@@ -200,7 +198,7 @@ static void handle_content_type(struct strbuf *line)
*content_top = boundary;
boundary = NULL;
}
-   slurp_attr(line->buf, "charset=", );
+   slurp_attr(line->buf, "charset=", >charset);
 
if (boundary) {
strbuf_release(boundary);
@@ -349,7 +347,7 @@ static int check_header(struct mailinfo *mi,
strbuf_add(, line->buf + len, line->len - len);
decode_header(mi, );
strbuf_insert(, 0, "Content-Type: ", len);
-   handle_content_type();
+   handle_content_type(mi, );
ret = 1;
goto check_header_out;
}
@@ -745,7 +743,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
strbuf *line)
mi->header_stage = 0;
 
/* normalize the log message to UTF-8. */
-   convert_to_utf8(mi, line, charset.buf);
+   convert_to_utf8(mi, line, mi->charset.buf);
 
if (mi->use_scissors && is_scissors_line(line)) {
int i;
@@ -840,7 +838,7 @@ again:
 
/* set some defaults */
mi->transfer_encoding = TE_DONTCARE;
-   strbuf_reset();
+   strbuf_reset(>charset);
 
/* slurp in this section's info */
while (read_one_header_line(line, mi->input))
@@ -1027,6 +1025,7 @@ static void setup_mailinfo(struct mailinfo *mi)
memset(mi, 0, sizeof(*mi));
strbuf_init(>name, 0);
strbuf_init(>email, 0);
+   strbuf_init(>charset, 0);
mi->header_stage = 1;
mi->use_inbody_headers = 1;
git_config(git_mailinfo_config, );
-- 
2.6.1-320-g86a1181

--
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: repository corruption when pushing commits to a repository running 'git gc --prune='

[Junio C Hamano]

Matthieu Moy  writes:

> This looks like a race between deleting objects/05/ and creating
> objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed.
>
> I don't understand how this is possible, since the temporary file and
> the final one are in the same directory, so deleting the directory
> should fail.
>
> What am I missing?

 1. tmp_obj_* gets created in objects/05/ by "git unpack-objects";

 2. prune_cruft() comes, notices tmp_obj_* that is sufficiently old,
decides to remove in "git prune".

 3. prune_subdir() comes, notices objects/05/ is now empty, does
rmdir().

 4. "git unpack-objects" closes the file whose inode was originally
associated with the path objects/05/tmp_obj_*;

 5. "git unpack-objects" calls finalize_object_file() and tries to
link(2) or rename(2) it to its final name.  It does not find the
source of the link or rename and says "oops, no such file".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 06/31] mailinfo: get rid of function-local static states

[Junio C Hamano]

Two helper functions use "static int" in their scope to keep track
of the state while repeatedly getting called once for each input
line.  Move these state variables their ultimate caller and pass
down pointers to them, as a small step in preparation for making
this entire callchain more reentrant.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 41 +++--
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 2b7f97b..1518708 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -713,27 +713,25 @@ static int is_scissors_line(const struct strbuf *line)
gap * 2 < perforation);
 }
 
-static int handle_commit_msg(struct strbuf *line)
+static int handle_commit_msg(struct strbuf *line, int *still_looking)
 {
-   static int still_looking = 1;
-
if (!cmitmsg)
return 0;
 
-   if (still_looking) {
+   if (*still_looking) {
if (!line->len || (line->len == 1 && line->buf[0] == '\n'))
return 0;
}
 
-   if (use_inbody_headers && still_looking) {
-   still_looking = check_header(line, s_hdr_data, 0);
-   if (still_looking)
+   if (use_inbody_headers && *still_looking) {
+   *still_looking = check_header(line, s_hdr_data, 0);
+   if (*still_looking)
return 0;
} else
/* Only trim the first (blank) line of the commit message
 * when ignoring in-body headers.
 */
-   still_looking = 0;
+   *still_looking = 0;
 
/* normalize the log message to UTF-8. */
if (metainfo_charset)
@@ -745,7 +743,7 @@ static int handle_commit_msg(struct strbuf *line)
die_errno("Could not rewind output message file");
if (ftruncate(fileno(cmitmsg), 0))
die_errno("Could not truncate output message file at 
scissors");
-   still_looking = 1;
+   *still_looking = 1;
 
/*
 * We may have already read "secondary headers"; purge
@@ -777,16 +775,13 @@ static void handle_patch(const struct strbuf *line)
patch_lines++;
 }
 
-static void handle_filter(struct strbuf *line)
+static void handle_filter(struct strbuf *line, int *filter_stage, int 
*header_stage)
 {
-   static int filter = 0;
-
-   /* filter tells us which part we left off on */
-   switch (filter) {
+   switch (*filter_stage) {
case 0:
-   if (!handle_commit_msg(line))
+   if (!handle_commit_msg(line, header_stage))
break;
-   filter++;
+   (*filter_stage)++;
case 1:
handle_patch(line);
break;
@@ -802,7 +797,7 @@ static int find_boundary(void)
return 0;
 }
 
-static int handle_boundary(void)
+static int handle_boundary(int *filter_stage, int *header_stage)
 {
struct strbuf newline = STRBUF_INIT;
 
@@ -824,7 +819,7 @@ again:
"can't recover\n");
exit(1);
}
-   handle_filter();
+   handle_filter(, filter_stage, header_stage);
strbuf_release();
 
/* skip to the next boundary */
@@ -852,6 +847,8 @@ again:
 static void handle_body(void)
 {
struct strbuf prev = STRBUF_INIT;
+   int filter_stage = 0;
+   int header_stage = 1;
 
/* Skip up to the first boundary */
if (*content_top) {
@@ -864,10 +861,10 @@ static void handle_body(void)
if (*content_top && is_multipart_boundary()) {
/* flush any leftover */
if (prev.len) {
-   handle_filter();
+   handle_filter(, _stage, 
_stage);
strbuf_reset();
}
-   if (!handle_boundary())
+   if (!handle_boundary(_stage, _stage))
goto handle_body_out;
}
 
@@ -897,7 +894,7 @@ static void handle_body(void)
strbuf_addbuf(, sb);
break;
}
-   handle_filter(sb);
+   handle_filter(sb, _stage, _stage);
}
/*
 * The partial chunk is saved in "prev" and will be
@@ -907,7 +904,7 @@ static void handle_body(void)
break;
}
default:
-   handle_filter();
+   handle_filter(, _stage, _stage);
}
 
} while 

[PATCH v2 05/31] mailinfo: move handle_boundary() lower

[Junio C Hamano]

This function wants to call find_boundary() and is called only from
one place without any recursing, so it becomes easier to read if it
appears after the called function.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 114 ++---
 1 file changed, 56 insertions(+), 58 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 73be47c..2b7f97b 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -626,64 +626,6 @@ static void decode_transfer_encoding(struct strbuf *line)
free(ret);
 }
 
-static void handle_filter(struct strbuf *line);
-
-static int find_boundary(void)
-{
-   while (!strbuf_getline(, fin, '\n')) {
-   if (*content_top && is_multipart_boundary())
-   return 1;
-   }
-   return 0;
-}
-
-static int handle_boundary(void)
-{
-   struct strbuf newline = STRBUF_INIT;
-
-   strbuf_addch(, '\n');
-again:
-   if (line.len >= (*content_top)->len + 2 &&
-   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
-   /* we hit an end boundary */
-   /* pop the current boundary off the stack */
-   strbuf_release(*content_top);
-   free(*content_top);
-   *content_top = NULL;
-
-   /* technically won't happen as is_multipart_boundary()
-  will fail first.  But just in case..
-*/
-   if (--content_top < content) {
-   fprintf(stderr, "Detected mismatched boundaries, "
-   "can't recover\n");
-   exit(1);
-   }
-   handle_filter();
-   strbuf_release();
-
-   /* skip to the next boundary */
-   if (!find_boundary())
-   return 0;
-   goto again;
-   }
-
-   /* set some defaults */
-   transfer_encoding = TE_DONTCARE;
-   strbuf_reset();
-
-   /* slurp in this section's info */
-   while (read_one_header_line(, fin))
-   check_header(, p_hdr_data, 0);
-
-   strbuf_release();
-   /* replenish line */
-   if (strbuf_getline(, fin, '\n'))
-   return 0;
-   strbuf_addch(, '\n');
-   return 1;
-}
-
 static inline int patchbreak(const struct strbuf *line)
 {
size_t i;
@@ -851,6 +793,62 @@ static void handle_filter(struct strbuf *line)
}
 }
 
+static int find_boundary(void)
+{
+   while (!strbuf_getline(, fin, '\n')) {
+   if (*content_top && is_multipart_boundary())
+   return 1;
+   }
+   return 0;
+}
+
+static int handle_boundary(void)
+{
+   struct strbuf newline = STRBUF_INIT;
+
+   strbuf_addch(, '\n');
+again:
+   if (line.len >= (*content_top)->len + 2 &&
+   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
+   /* we hit an end boundary */
+   /* pop the current boundary off the stack */
+   strbuf_release(*content_top);
+   free(*content_top);
+   *content_top = NULL;
+
+   /* technically won't happen as is_multipart_boundary()
+  will fail first.  But just in case..
+*/
+   if (--content_top < content) {
+   fprintf(stderr, "Detected mismatched boundaries, "
+   "can't recover\n");
+   exit(1);
+   }
+   handle_filter();
+   strbuf_release();
+
+   /* skip to the next boundary */
+   if (!find_boundary())
+   return 0;
+   goto again;
+   }
+
+   /* set some defaults */
+   transfer_encoding = TE_DONTCARE;
+   strbuf_reset();
+
+   /* slurp in this section's info */
+   while (read_one_header_line(, fin))
+   check_header(, p_hdr_data, 0);
+
+   strbuf_release();
+   /* replenish line */
+   if (strbuf_getline(, fin, '\n'))
+   return 0;
+   strbuf_addch(, '\n');
+   return 1;
+}
+
 static void handle_body(void)
 {
struct strbuf prev = STRBUF_INIT;
-- 
2.6.1-320-g86a1181

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 24/31] mailinfo: move read_one_header_line() closer to its callers

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 132 ++---
 1 file changed, 66 insertions(+), 66 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 138ca3b..0b083d0 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -390,72 +390,6 @@ check_header_out:
return ret;
 }
 
-static int is_rfc2822_header(const struct strbuf *line)
-{
-   /*
-* The section that defines the loosest possible
-* field name is "3.6.8 Optional fields".
-*
-* optional-field = field-name ":" unstructured CRLF
-* field-name = 1*ftext
-* ftext = %d33-57 / %59-126
-*/
-   int ch;
-   char *cp = line->buf;
-
-   /* Count mbox From headers as headers */
-   if (starts_with(cp, "From ") || starts_with(cp, ">From "))
-   return 1;
-
-   while ((ch = *cp++)) {
-   if (ch == ':')
-   return 1;
-   if ((33 <= ch && ch <= 57) ||
-   (59 <= ch && ch <= 126))
-   continue;
-   break;
-   }
-   return 0;
-}
-
-static int read_one_header_line(struct strbuf *line, FILE *in)
-{
-   /* Get the first part of the line. */
-   if (strbuf_getline(line, in, '\n'))
-   return 0;
-
-   /*
-* Is it an empty line or not a valid rfc2822 header?
-* If so, stop here, and return false ("not a header")
-*/
-   strbuf_rtrim(line);
-   if (!line->len || !is_rfc2822_header(line)) {
-   /* Re-add the newline */
-   strbuf_addch(line, '\n');
-   return 0;
-   }
-
-   /*
-* Now we need to eat all the continuation lines..
-* Yuck, 2822 header "folding"
-*/
-   for (;;) {
-   int peek;
-   struct strbuf continuation = STRBUF_INIT;
-
-   peek = fgetc(in); ungetc(peek, in);
-   if (peek != ' ' && peek != '\t')
-   break;
-   if (strbuf_getline(, in, '\n'))
-   break;
-   continuation.buf[0] = ' ';
-   strbuf_rtrim();
-   strbuf_addbuf(line, );
-   }
-
-   return 1;
-}
-
 static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
 {
const char *in = q_seg->buf;
@@ -794,6 +728,72 @@ static void handle_filter(struct mailinfo *mi, struct 
strbuf *line)
}
 }
 
+static int is_rfc2822_header(const struct strbuf *line)
+{
+   /*
+* The section that defines the loosest possible
+* field name is "3.6.8 Optional fields".
+*
+* optional-field = field-name ":" unstructured CRLF
+* field-name = 1*ftext
+* ftext = %d33-57 / %59-126
+*/
+   int ch;
+   char *cp = line->buf;
+
+   /* Count mbox From headers as headers */
+   if (starts_with(cp, "From ") || starts_with(cp, ">From "))
+   return 1;
+
+   while ((ch = *cp++)) {
+   if (ch == ':')
+   return 1;
+   if ((33 <= ch && ch <= 57) ||
+   (59 <= ch && ch <= 126))
+   continue;
+   break;
+   }
+   return 0;
+}
+
+static int read_one_header_line(struct strbuf *line, FILE *in)
+{
+   /* Get the first part of the line. */
+   if (strbuf_getline(line, in, '\n'))
+   return 0;
+
+   /*
+* Is it an empty line or not a valid rfc2822 header?
+* If so, stop here, and return false ("not a header")
+*/
+   strbuf_rtrim(line);
+   if (!line->len || !is_rfc2822_header(line)) {
+   /* Re-add the newline */
+   strbuf_addch(line, '\n');
+   return 0;
+   }
+
+   /*
+* Now we need to eat all the continuation lines..
+* Yuck, 2822 header "folding"
+*/
+   for (;;) {
+   int peek;
+   struct strbuf continuation = STRBUF_INIT;
+
+   peek = fgetc(in); ungetc(peek, in);
+   if (peek != ' ' && peek != '\t')
+   break;
+   if (strbuf_getline(, in, '\n'))
+   break;
+   continuation.buf[0] = ' ';
+   strbuf_rtrim();
+   strbuf_addbuf(line, );
+   }
+
+   return 1;
+}
+
 static int find_boundary(struct mailinfo *mi, struct strbuf *line)
 {
while (!strbuf_getline(line, mi->input, '\n')) {
-- 
2.6.1-320-g86a1181

--
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] merge: fix cache_entry use-after-free

[Junio C Hamano]

David Turner  writes:

> + unsigned int ref_count; /* count the number of refs to this in dir_hash 
> */

Me makes a mental note of the type used...

> @@ -213,6 +214,32 @@ struct cache_entry {
>  struct pathspec;
>  
>  /*
> + * Increment the cache_entry reference count.  Should be called
> + * whenever a pointer to a cache_entry is retained in a data structure,
> + * thus marking it as alive.
> + */
> +static inline void add_ce_ref(struct cache_entry *ce)
> +{
> + assert(ce != NULL && ce->ref_count >= 0);

... and notices that ce->ref_count will always be non-negative here


> + ce->ref_count++;
> +}
> +
> +/*
> + * Decrement the cache_entry reference count.  Should be called whenever
> + * a pointer to a cache_entry is dropped.  Once the counter drops to 0
> + * the cache_entry memory will be safely freed.
> + */
> +static inline void drop_ce_ref(struct cache_entry *ce)
> +{
> + if (ce != NULL) {
> + assert(ce->ref_count >= 0);

... and here.

By not checking integer overflow/wraparound, the code is assuming
that a ce entry will never referenced more than 4 billion times on
32-bit platform.  And that is a sensible assumption as there aren't
that many pointers in the address space to make that many reference
anyway.

Perhaps the code can assume the number won't be more than 2 billion
and use a signed type instead for the reference counting?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Tracing commands includes its working directory

[Stefan Beller]

Specially when having concurrent flows of command execution,
the order of commands printed cannot be relied upon. To aid
the poor developer follow the flow of commands include the
current working directory in which the command is carried out.

Signed-off-by: Stefan Beller 
---
 exec_cmd.c| 2 +-
 git.c | 8 
 run-command.c | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index e85f0fd..bb79933 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -125,7 +125,7 @@ const char **prepare_git_cmd(const char **argv)
 
 int execv_git_cmd(const char **argv) {
const char **nargv = prepare_git_cmd(argv);
-   trace_argv_printf(nargv, "trace: exec:");
+   trace_argv_printf(nargv, "(%s) trace: exec:", xgetcwd());
 
/* execvp() can only ever return if it fails */
sane_execvp("git", (char **)nargv);
diff --git a/git.c b/git.c
index deecba0..e077e2d 100644
--- a/git.c
+++ b/git.c
@@ -278,8 +278,8 @@ static int handle_alias(int *argcp, const char ***argv)
die("recursive alias: %s", alias_command);
 
trace_argv_printf(new_argv,
- "trace: alias expansion: %s =>",
- alias_command);
+ "(%s) trace: alias expansion: %s =>",
+ xgetcwd(), alias_command);
 
REALLOC_ARRAY(new_argv, count + *argcp);
/* insert after command name */
@@ -345,7 +345,7 @@ static int run_builtin(struct cmd_struct *p, int argc, 
const char **argv)
if (!help && p->option & NEED_WORK_TREE)
setup_work_tree();
 
-   trace_argv_printf(argv, "trace: built-in: git");
+   trace_argv_printf(argv, "(%s) trace: built-in: git", xgetcwd());
 
status = p->fn(argc, argv, prefix);
if (status)
@@ -557,7 +557,7 @@ static void execv_dashed_external(const char **argv)
tmp = argv[0];
argv[0] = cmd.buf;
 
-   trace_argv_printf(argv, "trace: exec:");
+   trace_argv_printf(argv, "(%s) trace: exec:", xgetcwd());
 
/*
 * if we fail because the command is not found, it is
diff --git a/run-command.c b/run-command.c
index ef3da27..e483bae 100644
--- a/run-command.c
+++ b/run-command.c
@@ -194,7 +194,7 @@ static const char **prepare_shell_cmd(const char **argv)
 static int execv_shell_cmd(const char **argv)
 {
const char **nargv = prepare_shell_cmd(argv);
-   trace_argv_printf(nargv, "trace: exec:");
+   trace_argv_printf(nargv, "(%s) trace: exec:", xgetcwd());
sane_execvp(nargv[0], (char **)nargv);
free(nargv);
return -1;
@@ -346,7 +346,7 @@ fail_pipe:
cmd->err = fderr[0];
}
 
-   trace_argv_printf(cmd->argv, "trace: run_command:");
+   trace_argv_printf(cmd->argv, "(%s) trace: run_command:", xgetcwd());
fflush(NULL);
 
 #ifndef GIT_WINDOWS_NATIVE
-- 
2.5.0.267.g8d6e698.dirty

--
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

What's cooking in git.git (Oct 2015, #03; Wed, 14)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

With somewhat reduced review bandwidth, I'd expect that the upcoming
cycle would be slower than usual.  At tinyurl.com/gitCal, I
tentatively drew a 14-week schedule for this cycle (I plan to be
offline during weeks #7-#9 myself---hopefully we'll have capable
interim maintainers to take care of the list traffic in the meantime
as in past years).

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* jk/asciidoctor-section-heading-markup-fix (2015-09-25) 1 commit
  (merged to 'next' on 2015-10-07 at 8bbff49)
 + Documentation: fix section header mark-up


* jk/notes-dwim-doc (2015-09-22) 1 commit
  (merged to 'next' on 2015-10-07 at c4341d1)
 + notes: correct documentation of DWIMery for notes references
 (this branch is used by mh/notes-allow-reading-treeish.)

 The way how --ref/--notes to specify the notes tree reference are
 DWIMmed was not clearly documented.


* nd/ls-remote-does-not-have-u-option (2015-09-28) 1 commit
  (merged to 'next' on 2015-10-07 at 0790a76)
 + ls-remote.txt: delete unsupported option


* pt/pull-builtin (2015-10-02) 1 commit
  (merged to 'next' on 2015-10-07 at 19af20e)
 + merge: grammofix in please-commit-before-merge message


* tk/typofix-connect-unknown-proto-error (2015-09-25) 1 commit
  (merged to 'next' on 2015-10-07 at cc3430e)
 + connect: fix typo in result string of prot_name()

--
[New Topics]

* jc/mailinfo (2015-10-08) 1 commit
 . mailinfo: ignore in-body header that we do not care about

 Some people write arbitrary garbage at the beginning of a piece of
 e-mail (or after -- >8 -- scissors -- >8 -- line) in the commit log
 message and expect them to be discarded, even though "From:" and
 "Subject:" are the only documented in-body headers that you are
 supposed to have there.  Allow some garbage (specifically, what may
 look like RFC2822 headers like "MIME-Version: ...") to be there and
 ignore them.

 I have a feeling that that this is a step in a wrong direction.
 Excluded from 'pu' for now.


* jk/filter-branch-use-of-sed-on-incomplete-line (2015-10-12) 1 commit
 - filter-branch: remove multi-line headers in msg filter

 A recent "filter-branch --msg-filter" broke skipping of the commit
 object header, which is fixed.

 Will merge to 'next'.


* rd/test-path-utils (2015-10-08) 1 commit
 - test-path-utils.c: remove incorrect assumption

 The normalize_ceiling_entry() function does not muck with the end
 of the path it accepts, and the real world callers do rely on that,
 but a test insisted that the function drops a trailing slash.

 Will merge to 'next'.


* jc/doc-gc-prune-now (2015-10-14) 1 commit
 - Documentation/gc: warn against --prune=

 "git gc" is safe to run anytime only because it has the built-in
 grace period to protect young objects.  In order to run with no
 grace period, the user must make sure that the repository is
 quiescent.

 Will merge to 'next'.


* jc/am-mailinfo-direct (2015-10-14) 1 commit
 - am: make direct call to mailinfo
 (this branch uses jc/mailinfo-lib.)

 "git am" used to spawn "git mailinfo" via run_command() API once
 per each patch, but learned to make a direct call to mailinfo()
 instead.

 Needs benchmark on platforms with slow run_command().


* jc/mailinfo-lib (2015-10-14) 30 commits
 - mailinfo: remove calls to exit() and die() deep in the callchain
 - mailinfo: handle charset conversion errors in the caller
 - mailinfo: libify
 - mailinfo: move definition of MAX_HDR_PARSED to closer to its use
 - mailinfo: move cleanup_space() before its users
 - mailinfo: move check_header() after the helpers it uses
 - mailinfo: move read_one_header_line() closer to its callers
 - mailinfo: move content/content_top to struct mailinfo
 - mailinfo: keep the parsed log message in a strbuf
 - mailinfo: move [ps]_hdr_data to struct mailinfo
 - mailinfo: move cmitmsg and patchfile to struct mailinfo
 - mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak
 - mailinfo: move charset to struct mailinfo
 - mailinfo: move transfer_encoding to struct mailinfo
 - mailinfo: move metainfo_charset to struct mailinfo
 - mailinfo: move use_scissors and use_inbody_headers to struct mailinfo
 - mailinfo: move add_message_id and message_id to struct mailinfo
 - mailinfo: move patch_lines to struct mailinfo
 - mailinfo: move filter/header stage to struct mailinfo
 - mailinfo: move global "FILE *fin, *fout" to struct mailinfo
 - mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo
 - mailinfo: introduce "struct mailinfo" to hold globals
 - mailinfo: move global "line" into mailinfo() function
 - mailinfo: always pass "line" as an 

Re: [PATCH v3 1/3] Add Travis CI support

[Lars Schneider]

On 12 Oct 2015, at 22:20, Matthieu Moy  wrote:

> larsxschnei...@gmail.com writes:
> 
>> --- /dev/null
>> +++ b/.travis.yml
>> @@ -0,0 +1,46 @@
>> +language: c
>> +
>> +os:
>> +  - linux
>> +  - osx
>> +
>> +compiler:
>> +  - clang
>> +  - gcc
>> +
>> +before_install:
>> +  - >
>> +export GIT_TEST_OPTS=" --quiet";
>> +case "${TRAVIS_OS_NAME:-linux}" in
>> +linux)
>> +  wget -q https://package.perforce.com/perforce.pubkey -O - \
>> +| sudo apt-key add -
>> +  echo 'deb http://package.perforce.com/apt/ubuntu precise release' \
>> +| sudo tee -a /etc/apt/sources.list
>> +  wget -q https://packagecloud.io/gpg.key -O - | sudo apt-key add -
>> +  echo 'deb https://packagecloud.io/github/git-lfs/debian/ jessie main' 
>> \
>> +| sudo tee -a /etc/apt/sources.list
>> +  sudo apt-get update -qq
>> +  sudo apt-get install -y apt-transport-https
>> +  sudo apt-get install perforce-server git-lfs
> 
> Sorry if this has been discussed already, but do you really need these
> "sudo" calls?
> 
> They trigger builds on the legacy Travis CI infrastructure:
> 
>  
> http://docs.travis-ci.com/user/migrating-from-legacy/?utm_source=legacy-notice_medium=banner_campaign=legacy-upgrade
> 
> No big deal, but getting rid of sudo would be cool, and documenting why
> it can't easily be done in commit message and/or comments would be nice
> otherwise.
I would like to get rid of the "sudo" calls, too. Unfortunately I wasn't able 
to achieve this so far because these packages are not white listed on Travis CI 
(see Jean-Noël answer in this thread). I tried to download and install the 
*.deb files manually using dpkg without luck. Any idea or hint?
Nevertheless I don't think this is a problem as Travis CI states that "sudo 
isn't possible (__right now__)" on the new infrastructure. They need to find a 
solutions because I believe many projects will run into this issue...
http://docs.travis-ci.com/user/migrating-from-legacy/?utm_source=legacy-notice_medium=banner_campaign=legacy-upgrade#Using-sudo-isn%E2%80%99t-possible-(right-now)

- Lars--
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: repository corruption when pushing commits to a repository running 'git gc --prune='

[Matthieu Moy]

Junio C Hamano  writes:

> Matthieu Moy  writes:
>
>> Looks good to me. I think the same should be added to git-prune.txt
>> under --expire.
>
> I thought about it, but decided against it, as the command is not
> even recommended to end users.

Even non-"end users" can shoot themselves in the foot. Someone writting
a script using "git prune" may actually shoot in other people's foot ...

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: repository corruption when pushing commits to a repository running 'git gc --prune='

[Junio C Hamano]

Matthieu Moy  writes:

> Junio C Hamano  writes:
>
>> Matthieu Moy  writes:
>>
>>> Looks good to me. I think the same should be added to git-prune.txt
>>> under --expire.
>>
>> I thought about it, but decided against it, as the command is not
>> even recommended to end users.
>
> Even non-"end users" can shoot themselves in the foot. Someone writting
> a script using "git prune" may actually shoot in other people's foot ...

Sure, patches welcome.

To be honest, I felt that

--expire ::
Only expire loose objects older than .

is sufficiently clear for these people.
--
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] merge: fix cache_entry use-after-free

[David Turner]

On Wed, 2015-10-14 at 14:17 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> > +   unsigned int ref_count; /* count the number of refs to this in dir_hash 
> > */
> 
> Me makes a mental note of the type used...
> 
> > @@ -213,6 +214,32 @@ struct cache_entry {
> >  struct pathspec;
> >  
> >  /*
> > + * Increment the cache_entry reference count.  Should be called
> > + * whenever a pointer to a cache_entry is retained in a data structure,
> > + * thus marking it as alive.
> > + */
> > +static inline void add_ce_ref(struct cache_entry *ce)
> > +{
> > +   assert(ce != NULL && ce->ref_count >= 0);
> 
> ... and notices that ce->ref_count will always be non-negative here
> 
> 
> > +   ce->ref_count++;
> > +}
> > +
> > +/*
> > + * Decrement the cache_entry reference count.  Should be called whenever
> > + * a pointer to a cache_entry is dropped.  Once the counter drops to 0
> > + * the cache_entry memory will be safely freed.
> > + */
> > +static inline void drop_ce_ref(struct cache_entry *ce)
> > +{
> > +   if (ce != NULL) {
> > +   assert(ce->ref_count >= 0);
> 
> ... and here.
> 
> By not checking integer overflow/wraparound, the code is assuming
> that a ce entry will never referenced more than 4 billion times on
> 32-bit platform.  And that is a sensible assumption as there aren't
> that many pointers in the address space to make that many reference
> anyway.
> 
> Perhaps the code can assume the number won't be more than 2 billion
> and use a signed type instead for the reference counting?

Will do.

--
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] Tracing commands includes its working directory

[Junio C Hamano]

Stefan Beller  writes:

> Specially when having concurrent flows of command execution,
> the order of commands printed cannot be relied upon. To aid
> the poor developer follow the flow of commands include the
> current working directory in which the command is carried out.
>
> Signed-off-by: Stefan Beller 
> ---

I am not quite sure about the motivation behind this patch.  If you
are doing N processes, each of which *happens* to be working inside
its own directory that is different from all others, then it may be
a usable way to tell them apart, but that smells like too much of
special casing that will be applicable ohly to what you happen to be
developing recently and nowhere else.  Some of these N processes
share the same cwd in an application different from yours.

One way to tweak it to be generic enough may be to show a trace
output when git goes into a different working directory, which is a
significant event that is worth tracing, and then have a mode where
all trace lines are prefixed with process ID, perhaps?

>  exec_cmd.c| 2 +-
>  git.c | 8 
>  run-command.c | 4 ++--
>  3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/exec_cmd.c b/exec_cmd.c
> index e85f0fd..bb79933 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -125,7 +125,7 @@ const char **prepare_git_cmd(const char **argv)
>  
>  int execv_git_cmd(const char **argv) {
>   const char **nargv = prepare_git_cmd(argv);
> - trace_argv_printf(nargv, "trace: exec:");
> + trace_argv_printf(nargv, "(%s) trace: exec:", xgetcwd());
>  
>   /* execvp() can only ever return if it fails */
>   sane_execvp("git", (char **)nargv);
> diff --git a/git.c b/git.c
> index deecba0..e077e2d 100644
> --- a/git.c
> +++ b/git.c
> @@ -278,8 +278,8 @@ static int handle_alias(int *argcp, const char ***argv)
>   die("recursive alias: %s", alias_command);
>  
>   trace_argv_printf(new_argv,
> -   "trace: alias expansion: %s =>",
> -   alias_command);
> +   "(%s) trace: alias expansion: %s =>",
> +   xgetcwd(), alias_command);
>  
>   REALLOC_ARRAY(new_argv, count + *argcp);
>   /* insert after command name */
> @@ -345,7 +345,7 @@ static int run_builtin(struct cmd_struct *p, int argc, 
> const char **argv)
>   if (!help && p->option & NEED_WORK_TREE)
>   setup_work_tree();
>  
> - trace_argv_printf(argv, "trace: built-in: git");
> + trace_argv_printf(argv, "(%s) trace: built-in: git", xgetcwd());
>  
>   status = p->fn(argc, argv, prefix);
>   if (status)
> @@ -557,7 +557,7 @@ static void execv_dashed_external(const char **argv)
>   tmp = argv[0];
>   argv[0] = cmd.buf;
>  
> - trace_argv_printf(argv, "trace: exec:");
> + trace_argv_printf(argv, "(%s) trace: exec:", xgetcwd());
>  
>   /*
>* if we fail because the command is not found, it is
> diff --git a/run-command.c b/run-command.c
> index ef3da27..e483bae 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -194,7 +194,7 @@ static const char **prepare_shell_cmd(const char **argv)
>  static int execv_shell_cmd(const char **argv)
>  {
>   const char **nargv = prepare_shell_cmd(argv);
> - trace_argv_printf(nargv, "trace: exec:");
> + trace_argv_printf(nargv, "(%s) trace: exec:", xgetcwd());
>   sane_execvp(nargv[0], (char **)nargv);
>   free(nargv);
>   return -1;
> @@ -346,7 +346,7 @@ fail_pipe:
>   cmd->err = fderr[0];
>   }
>  
> - trace_argv_printf(cmd->argv, "trace: run_command:");
> + trace_argv_printf(cmd->argv, "(%s) trace: run_command:", xgetcwd());
>   fflush(NULL);
>  
>  #ifndef GIT_WINDOWS_NATIVE
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] merge: fix cache_entry use-after-free

[David Turner]

From: Keith McGuigan 

During merges, we would previously free entries that we no longer need
in the destination index.  But those entries might also be stored in
the dir_entry cache, and when a later call to add_to_index found them,
they would be used after being freed.

To prevent this, add a ref count for struct cache_entry.  Whenever
a cache entry is added to a data structure, the ref count is incremented;
when it is removed from the data structure, it is decremented.  When
it hits zero, the cache_entry is freed.

Signed-off-by: Keith McGuigan 
Signed-off-by: David Turner 
---

Fix type of ref_count (from unsigned int to int).


 cache.h| 27 +++
 name-hash.c|  7 ++-
 read-cache.c   |  6 +-
 split-index.c  | 13 -
 unpack-trees.c |  6 --
 5 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/cache.h b/cache.h
index 752031e..7906026 100644
--- a/cache.h
+++ b/cache.h
@@ -149,6 +149,7 @@ struct stat_data {
 
 struct cache_entry {
struct hashmap_entry ent;
+   int ref_count; /* count the number of refs to this in dir_hash */
struct stat_data ce_stat_data;
unsigned int ce_mode;
unsigned int ce_flags;
@@ -213,6 +214,32 @@ struct cache_entry {
 struct pathspec;
 
 /*
+ * Increment the cache_entry reference count.  Should be called
+ * whenever a pointer to a cache_entry is retained in a data structure,
+ * thus marking it as alive.
+ */
+static inline void add_ce_ref(struct cache_entry *ce)
+{
+   assert(ce != NULL && ce->ref_count >= 0);
+   ce->ref_count++;
+}
+
+/*
+ * Decrement the cache_entry reference count.  Should be called whenever
+ * a pointer to a cache_entry is dropped.  Once the counter drops to 0
+ * the cache_entry memory will be safely freed.
+ */
+static inline void drop_ce_ref(struct cache_entry *ce)
+{
+   if (ce != NULL) {
+   assert(ce->ref_count >= 0);
+   if (--ce->ref_count < 1) {
+   free(ce);
+   }
+   }
+}
+
+/*
  * Copy the sha1 and stat state of a cache entry from one to
  * another. But we never change the name, or the hash state!
  */
diff --git a/name-hash.c b/name-hash.c
index 702cd05..f12c919 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -66,6 +66,7 @@ static struct dir_entry *hash_dir_entry(struct index_state 
*istate,
dir = xcalloc(1, sizeof(struct dir_entry));
hashmap_entry_init(dir, memihash(ce->name, namelen));
dir->namelen = namelen;
+   add_ce_ref(ce);
dir->ce = ce;
hashmap_add(>dir_hash, dir);
 
@@ -92,7 +93,9 @@ static void remove_dir_entry(struct index_state *istate, 
struct cache_entry *ce)
struct dir_entry *dir = hash_dir_entry(istate, ce, ce_namelen(ce));
while (dir && !(--dir->nr)) {
struct dir_entry *parent = dir->parent;
-   hashmap_remove(>dir_hash, dir, NULL);
+   struct dir_entry *removed = hashmap_remove(>dir_hash, 
dir, NULL);
+   assert(removed == dir);
+   drop_ce_ref(dir->ce);
free(dir);
dir = parent;
}
@@ -105,6 +108,7 @@ static void hash_index_entry(struct index_state *istate, 
struct cache_entry *ce)
ce->ce_flags |= CE_HASHED;
hashmap_entry_init(ce, memihash(ce->name, ce_namelen(ce)));
hashmap_add(>name_hash, ce);
+   add_ce_ref(ce);
 
if (ignore_case)
add_dir_entry(istate, ce);
@@ -147,6 +151,7 @@ void remove_name_hash(struct index_state *istate, struct 
cache_entry *ce)
return;
ce->ce_flags &= ~CE_HASHED;
hashmap_remove(>name_hash, ce, ce);
+   drop_ce_ref(ce);
 
if (ignore_case)
remove_dir_entry(istate, ce);
diff --git a/read-cache.c b/read-cache.c
index 87204a5..8b685bb 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -52,7 +52,9 @@ static const char *alternate_index_output;
 
 static void set_index_entry(struct index_state *istate, int nr, struct 
cache_entry *ce)
 {
+   /* istate->cache[nr] is assumed to not hold a live value */
istate->cache[nr] = ce;
+   add_ce_ref(ce);
add_name_hash(istate, ce);
 }
 
@@ -62,7 +64,7 @@ static void replace_index_entry(struct index_state *istate, 
int nr, struct cache
 
replace_index_entry_in_base(istate, old, ce);
remove_name_hash(istate, old);
-   free(old);
+   drop_ce_ref(old);
set_index_entry(istate, nr, ce);
ce->ce_flags |= CE_UPDATE_IN_BASE;
istate->cache_changed |= CE_ENTRY_CHANGED;
@@ -75,6 +77,7 @@ void rename_index_entry_at(struct index_state *istate, int 
nr, const char *new_n
 
new = xmalloc(cache_entry_size(namelen));
copy_cache_entry(new, old);
+   new->ref_count = 0;
new->ce_flags &= ~CE_HASHED;
new->ce_namelen = namelen;

Re: [PATCH v2 20/31] mailinfo: move cmitmsg and patchfile to struct mailinfo

[Junio C Hamano]

This step does not compile correctly; will be fixed in v3.
--
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 21/31] mailinfo: move [ps]_hdr_data to struct mailinfo

[Junio C Hamano]

Junio C Hamano  writes:

> @@ -995,10 +995,10 @@ static int mailinfo(struct mailinfo *mi, const char 
> *msg, const char *patch)
>  
>   /* process the email header */
>   while (read_one_header_line(, mi->input))
> - check_header(mi, , p_hdr_data, 1);
> + check_header(mi, , mi->p_hdr_data, 1);
>  
>   handle_body(mi, );
> - fclose(patchfile);
> + fclose(mi->patchfile);

... this is the bit that should have be in 20/31.
--
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] git-gui: Update Russian translation

[Pat Thoyts]

Dimitriy Ryazantcev  writes:

>Signed-off-by: Dimitriy Ryazantcev 
>---
> po/ru.po | 675 +++
> 1 file changed, 247 insertions(+), 428 deletions(-)
>
>diff --git a/po/ru.po b/po/ru.po
>index ca4343b..8ebc98f 100644
>--- a/po/ru.po
>+++ b/po/ru.po
>@@ -1,19 +1,23 @@
>-# Translation of git-gui to russian
>-# Copyright (C) 2007 Shawn Pearce
>-# This file is distributed under the same license as the git-gui package.
>-# Irina Riesen , 2007.
>-#
>+# SOME DESCRIPTIVE TITLE.
>+# Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER
>+# This file is distributed under the same license as the PACKAGE package.


This section seems wrong as it replaces the existing copyright notice
with some auto-generated junk. As it is a translation I should think the
copyright is with the translators but it should not be using default
placeholders.

>+# 
>+# Translators:
>+# Dimitriy Ryazantcev , 2015
>+# Irina Riesen , 2007
> msgid ""
> msgstr ""
>-"Project-Id-Version: git-gui\n"
>+"Project-Id-Version: Git Russian Localization Project\n"
> "Report-Msgid-Bugs-To: \n"
> "POT-Creation-Date: 2010-01-26 15:47-0800\n"
>-"PO-Revision-Date: 2007-10-22 22:30-0200\n"
>-"Last-Translator: Alex Riesen \n"
>-"Language-Team: Russian Translation \n"
>+"PO-Revision-Date: 2015-10-12 11:36+\n"
>+"Last-Translator: Dimitriy Ryazantcev \n"
>+"Language-Team: Russian 
>(http://www.transifex.com/djm00n/git-po-ru/language/ru/)\n"
> "MIME-Version: 1.0\n"
> "Content-Type: text/plain; charset=UTF-8\n"
> "Content-Transfer-Encoding: 8bit\n"
>+"Language: ru\n"
>+"Plural-Forms: nplurals=4; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2
>&& n%10<=4 && (n%100<12 || n%100>14) ? 1 : n%10==0 || (n%10>=5 &&
>n%10<=9) || (n%100>=11 && n%100<=14)? 2 : 3);\n"
> 
[snip]

-- 
Pat Thoytshttp://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD
--
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: thread-utils: build with NO_PTHREADS fails

[Victor Leschuk]

Hello Junio, 

sorry that was my fault, I was building it wrong way (defined NO_PTHREADS in 
CFLAGS variable, not as separate make variable). Sorry for the false alarm.

--
Best Regards,
Victor

From: Junio C Hamano [jch2...@gmail.com] On Behalf Of Junio C Hamano 
[gits...@pobox.com]
Sent: Monday, October 12, 2015 10:55 AM
To: Victor Leschuk
Cc: git@vger.kernel.org; vlesc...@gmail.com
Subject: Re: thread-utils: build with NO_PTHREADS fails

Junio C Hamano  writes:

> Victor Leschuk  writes:
>
>> I think that no one tried it for a long time but I needed a
>> single-threaded git version for debug purpose. I tried to build
>> with -DNO_PTHREADS and thread-utils.c failed to compile.
>>
>> In brief the situation is the following:
>>
>> in header file we have something like that:
>>
>>
>> #ifndef NO_PTHREAD
>> extern int online_cpus(void);
>>
>> #else
>> #define online_cpus() 1
>> #endif // NO_PTHREAD
>>
>> and in *.c file:
>>
>>
>> int online_cpus(void)
>> {
>> // ...
>> }
>
> Yeah, that is obviously incorrect.
> ...

Well, no, I spoke too early.  I do not see there is much wrong here.

There is this bit in the Makefile:

ifdef NO_PTHREADS
BASIC_CFLAGS += -DNO_PTHREADS
else
BASIC_CFLAGS += $(PTHREAD_CFLAGS)
EXTLIBS += $(PTHREAD_LIBS)
LIB_OBJS += thread-utils.o
endif

The source file thread-utils.c is not compiled to thread-utils.o if
you say NO_PTHREADS, and the resulting libgit.a does not of course
have it.
--
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: [BUG] Broken links in Git Documentation/user-manual.txt

[John Keeping]

On Wed, Oct 14, 2015 at 09:37:05AM +0200, Matthieu Moy wrote:
> Xue Fuqiao  writes:
> 
> > Hi list,
> >
> > In https://git-scm.com/docs/user-manual.html , all links to the
> > glossary[1] are broken.
> 
> Actually, the links themselves are fine, but the destimation is broken.
> 
> The doc is supposed to look like this :
> 
>   https://www.kernel.org/pub/software/scm/git/docs/user-manual.html#def_head
> 
> with the glossary at the end. On
> https://git-scm.com/docs/user-manual.html, the glossary is displayed as
> verbatim text.
> 
> This does not seem to be a bug in our user-manual.txt, but in the way
> it's processed by git-scm.com.

I think it was an issue in the source, but was fixed by be510e0
(Documentation: fix section header mark-up, 2015-09-25).  I'm not sure
when/how git-scm.com rebulds its documentation, but I'm pretty sure that
fix hasn't made it into a release yet so I doubt the site has picked it
up.

> I reported the issue there:
> 
> https://github.com/git/git-scm.com/issues/605
--
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 v3] merge: fix cache_entry use-after-free

[René Scharfe]

Am 15.10.2015 um 00:07 schrieb David Turner:

From: Keith McGuigan 

During merges, we would previously free entries that we no longer need
in the destination index.  But those entries might also be stored in
the dir_entry cache, and when a later call to add_to_index found them,
they would be used after being freed.

To prevent this, add a ref count for struct cache_entry.  Whenever
a cache entry is added to a data structure, the ref count is incremented;
when it is removed from the data structure, it is decremented.  When
it hits zero, the cache_entry is freed.

Signed-off-by: Keith McGuigan 
Signed-off-by: David Turner 
---

Fix type of ref_count (from unsigned int to int).


  cache.h| 27 +++
  name-hash.c|  7 ++-
  read-cache.c   |  6 +-
  split-index.c  | 13 -
  unpack-trees.c |  6 --
  5 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/cache.h b/cache.h
index 752031e..7906026 100644
--- a/cache.h
+++ b/cache.h
@@ -149,6 +149,7 @@ struct stat_data {

  struct cache_entry {
struct hashmap_entry ent;
+   int ref_count; /* count the number of refs to this in dir_hash */
struct stat_data ce_stat_data;
unsigned int ce_mode;
unsigned int ce_flags;
@@ -213,6 +214,32 @@ struct cache_entry {
  struct pathspec;

  /*
+ * Increment the cache_entry reference count.  Should be called
+ * whenever a pointer to a cache_entry is retained in a data structure,
+ * thus marking it as alive.
+ */
+static inline void add_ce_ref(struct cache_entry *ce)
+{
+   assert(ce != NULL && ce->ref_count >= 0);
+   ce->ref_count++;
+}
+
+/*
+ * Decrement the cache_entry reference count.  Should be called whenever
+ * a pointer to a cache_entry is dropped.  Once the counter drops to 0
+ * the cache_entry memory will be safely freed.
+ */
+static inline void drop_ce_ref(struct cache_entry *ce)
+{
+   if (ce != NULL) {
+   assert(ce->ref_count >= 0);


Shouldn't this be "> 0" to prevent double frees?


+   if (--ce->ref_count < 1) {
+   free(ce);
+   }
+   }
+}


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Falis on commit --amend when already pushed

[Tomohiro Koana]

Hello all,

I'm a third year student at the University of Tokyo and, in our
"Diving into open-source software" class, my friends and I decided to
work with git. Our final, hopefully, is contributing to git.

One improvement that we thought of was not letting users to amend
commit when the commit is already pushed to the remote server.

--- a/builtin/commit.c

+++ b/builtin/commit.c

@@ -32,6 +32,7 @@

 #include "sequencer.h"

 #include "notes-utils.h"

 #include "mailmap.h"

+#include "remote.h"



 static const char * const builtin_commit_usage[] = {

  N_("git commit [] [--] ..."),

@@ -1125,6 +1126,9 @@ static int parse_and_validate_options(int argc,
const char *argv[],

struct wt_status *s)

 {

  int f = 0;

+ int ours, theirs;

+ const char *full_base;

+ struct branch *branch = branch_get(NULL);



  argc = parse_options(argc, argv, prefix, options, usage, 0);

  finalize_deferred_config(s);

@@ -1149,6 +1153,12 @@ static int parse_and_validate_options(int argc,
const char *argv[],

  else if (whence == FROM_CHERRY_PICK)

  die(_("You are in the middle of a cherry-pick -- cannot amend."));

  }

+

+ stat_tracking_info(branch, , , _base);

+ if (amend && ours == 0) {

+ die(_("This commit is already pushed to the remote -- cannot amend."));

+ }

+

  if (fixup_message && squash_message)

  die(_("Options --squash and --fixup cannot be used together"));

  if (use_message)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] git-gui: Update Russian translation

[Dimitriy Ryazantcev]

>This section seems wrong as it replaces the existing copyright notice
>with some auto-generated junk. As it is a translation I should think the
>copyright is with the translators but it should not be using default
>placeholders.
Sorry for that. It seems that a Transifex bug, I uploaded po with copyrights
to them. Sent a mail to Transifex support. For now I replaced it manually.

Dimitriy Ryazantcev (1):
  git-gui: Update Russian translation

 po/ru.po | 668 +++
 1 file changed, 243 insertions(+), 425 deletions(-)

-- 
2.6.0

--
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

[Bug?] .git visible in git-status

[Radosław Warzocha]

I've recently stumbled upon interesting issue - .git/* files occurring
in the git-status output.
Let me elaborate:
1. When running
> git status -s
no .git/* files are displayed
2. When running:
> git status -s .git
only an empty line is printed out
3. But when I run (for example):
> git status -s .git/HEAD
I get
?? .git/HEAD
Files are listed also when -s option is not present. When providing
.git/* all of the files under .git directory are printed.

I don't know if this is desired behavior, but i guess it's not.

Best regards
Radek
--
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

Captain Kelvin Ken Miller

Am Captain Kelvin Ken Miller i am with the us army in Camp Abu Naji / FOB Garry 
Owen (Al Amarah)I need you assistant to move some funds out of Iraq. 
Kindly respond for more details.
--
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

repository corruption when pushing commits to a repository running 'git gc --prune='

[Jan Smets]

Hi

I've recently expired my reflog to prune loose objects. On a live, bare, 
repository I ran 'git gc --prune=now'


All clients ended up having problems, they would report:
 error: refs/heads/master does not point to a valid object!
Running 'git log' on the bare repo gave : fatal: bad object HEAD



So I did a couple of tries reproducing the issue and with this little 
test I got a couple of interesting different/similar outcomes


A: mkdir -p /tmp/test; cd /tmp/test ; git init --bare bare ;
B: cd /tmp/test ; git clone bare 1 ; cd 1;  touch test; git add test; 
while true; do date >> test ; git commit -m "$(date)" test ; git push || 
break; done

A: cd bare; git gc --prune=now  (or run it in a loop)


1)

fatal: bad object 22f0351258fa0bb4cd28984b6473510957fbce69
fatal: bad object 22f0351258fa0bb4cd28984b6473510957fbce69
To /tmp/test/bare
 ! [remote rejected] master -> master (missing necessary objects)

2)

remote: error: unable to write sha1 filename 
objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed: No such file or directory

remote: fatal: failed to write object
error: unpack failed: unpack-objects abnormal exit
To /tmp/test/bare
 ! [remote rejected] master -> master (unpacker error)

3)

error: Ref refs/heads/master is at 
e992810e70949e797d33041bf6bc961c9fa4f3e5 but expected 


remote: error: Cannot lock ref 'refs/heads/master':
To /tmp/test/bare
 ! [remote rejected] master -> master (failed to update ref)

4)

remote: error: cannot lock ref 'refs/heads/master': Unable to create 
'/tmp/test/bare/refs/heads/master.lock': File exists.

remote:
remote: If no other git process is currently running, this probably means a
remote: git process crashed in this repository earlier. Make sure no 
other git

remote: process is running and remove the file manually to continue.
To /tmp/test/bare
 ! [remote rejected] master -> master (failed to update ref)
error: failed to push some refs to '/tmp/test/bare'


And eventually running 'git gc --prune=now/all' on the bare repository 
could end with this message:


$ git gc --prune=all
error: refs/heads/master does not point to a valid object!
fatal: bad object refs/heads/master
error: failed to run repack
$ git log -1
fatal: bad object HEAD


This behaviour has been observed with version 2.4.4, 2.5.[12] and 2.6.1

Thank you

- Jan







--
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: Alternative to manual editing with git add --patch

[Sven Helmberger]

Am 14.10.2015 um 19:50 schrieb Junio C Hamano:
> Sven Helmberger  writes:
> 
> As a quick-and-dirty change, you could invent a new variant of
> 's'plit that breaks a N-line hunk into N hunks with 1-line each, but
> obviously that would not be a pleasant-enough UI to be called usable
> when you have a hunk that adds 100 lines.  Perhaps "Split this hunk
> into two by ending the earlier one immediately before the line that
> has this substring" or something might be an idea?
> 

If we go by the style of interaction in git add --patch and git add
--interactive, I think the most canonical solution would be to implement
it like this.

If we know when we can't split the current patch any further ( the point
at which selecting s changes nothing anymore), why shouldn't add --patch
not work similar to add --interactive in that it prints the lines of the
diff prefixed with numbers and the user can define a numerical range to
"split off". Then they decide whether to add those lines or not and
return to the line-numbers until they're trough with the patch.

Regards,
Sven Helmberger
--
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