* Re: Log message printout cleanups
From: Junio C Hamano @ 2006-04-18 5:52 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604171751540.3701@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> There's actually something _wrong_ with "git log --stat".
>
> What happens is that "git log" will enable "rev.combine_merges" by
> default, and that means that together with "--stat" ignoring merges by
> default, it will _instead_ generate a "--cc" diff for that merge.
>
> I'll leave that up to you, I'm getting pretty fed up with "git log" right
> now ;)
The primary problem is that the code in "next" was before the
"fancy diffstat for combined" work by Johannes; it punts and
always shows the patch output even when stat is asked for.
I think this does what both of us want. One thing I noticed is
that log-tree-diff-flush does "---\n" under --patch-with-stat
but not under --stat; I matched that here.
-- >8 --
combine-diff: show diffstat with the first parent.
Even under combine-diff, asking for stat (either with --stat or
--patch-with-stat) gives you diffstat for the first parent.
While the combined patch is useful to highlight the complexity
and interaction of the parts touched by all branches when
reviewing a merge commit, diffstat is a tool to assess the
extent of damage the merge brings in, and showing stat with the
first parent is more sensible than clever per-parent diffstat.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
combine-diff.c | 24 +++++++++++++++++++-----
1 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index aef9006..27f6f57 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -831,10 +831,11 @@ void show_combined_diff(struct combine_d
case DIFF_FORMAT_NAME:
show_raw_diff(p, num_parent, rev);
return;
-
- default:
case DIFF_FORMAT_PATCH:
show_patch_diff(p, num_parent, dense, rev);
+ return;
+ default:
+ return;
}
}
@@ -847,10 +848,13 @@ void diff_tree_combined_merge(const unsi
struct commit_list *parents;
struct combine_diff_path *p, *paths = NULL;
int num_parent, i, num_paths;
+ int do_diffstat;
+ do_diffstat = (opt->output_format == DIFF_FORMAT_DIFFSTAT ||
+ opt->with_stat);
diffopts = *opt;
- diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
diffopts.with_raw = 0;
+ diffopts.with_stat = 0;
diffopts.recursive = 1;
/* count parents */
@@ -864,14 +868,24 @@ void diff_tree_combined_merge(const unsi
parents;
parents = parents->next, i++) {
struct commit *parent = parents->item;
+ /* show stat against the first parent even
+ * when doing combined diff.
+ */
+ if (i == 0 && do_diffstat)
+ diffopts.output_format = DIFF_FORMAT_DIFFSTAT;
+ else
+ diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
diff_tree_sha1(parent->object.sha1, commit->object.sha1, "",
&diffopts);
diffcore_std(&diffopts);
paths = intersect_paths(paths, i, num_parent);
- if (diffopts.with_stat && rev->loginfo)
- show_log(rev, rev->loginfo, "---\n");
+ if (do_diffstat && rev->loginfo)
+ show_log(rev, rev->loginfo,
+ opt->with_stat ? "---\n" : "\n");
diff_flush(&diffopts);
+ if (opt->with_stat)
+ putchar('\n');
}
/* find out surviving paths */
^ permalink raw reply related
* Re: Log message printout cleanups
From: Linus Torvalds @ 2006-04-18 0:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v7j5nk9bf.fsf@assigned-by-dhcp.cox.net>
On Mon, 17 Apr 2006, Junio C Hamano wrote:
>
> These days I find myself running "git log --stat" more often
> than "git whatchanged"; it looks so much nicer ;-).
>
> Thanks for a job well done.
There's actually something _wrong_ with "git log --stat".
What happens is that "git log" will enable "rev.combine_merges" by
default, and that means that together with "--stat" ignoring merges by
default, it will _instead_ generate a "--cc" diff for that merge.
I think that behaviour was introduced by the "--stat" code by Dscho, and
not by my rewrite.
You can see it on the kernel archive on commit 88dd9c16, for example:
git log --stat 88dd9c16
will make it pretty obvious what I'm talking about.
I still haven't looked a lot at what the diffstat code does, so I don't
know what the obvious fix is. Maybe setting "ignore_merges". And maybe
just making the diffstat thing do the right thing for merges.
I'll leave that up to you, I'm getting pretty fed up with "git log" right
now ;)
Linus
^ permalink raw reply
* Re: Log message printout cleanups
From: Linus Torvalds @ 2006-04-18 0:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vejzvka09.fsf@assigned-by-dhcp.cox.net>
On Mon, 17 Apr 2006, Junio C Hamano wrote:
>
> Showing stat from the first parent makes --stat inconsistent
> from what the "fake diff" shows, but I think it is a sensible
> thing to do.
It might actually be even more than sensible: it's a huge hint especially
for users of "--cc" (or even --combined) that the diff we show really
isn't all there is to the merge. So having the "full" diffstat is not just
usually what you want to see, it's probably also a really good hint for
people who otherwise might not realize that our combined diffs have
skipped all the common parts.
Btw, I just noticed that I broke "--pretty=oneline" AGAIN. That thing is
very special, since pretty_print_commit() will _remove_ the newline at the
end of it, so we want to have an extra separator between the things.
I added a honking big comment this time, so that (a) I don't forget this
_again_ (I broke "oneline" several times during this printout cleanup),
and so that people can understand _why_ the code does what it does.
Now, arguably the alternate fix is to always have the '\n' at the end in
pretty-print-commit, but git-rev-list depends on the current behaviour
(but we could have git-rev-list remove it, whatever).
With the big comment, the code hopefully doesn't get broken again. And now
things like
git log --pretty=oneline --cc --patch-with-stat
works (even if that is admittedly a totally insane combination: if you
want the patch, having the "oneline" log format is just crazy, but hey,
it _works_. Even insane people are people).
Linus
---
diff --git a/log-tree.c b/log-tree.c
index c0a4432..9634c46 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -10,6 +9,7 @@ void show_log(struct rev_info *opt, stru
struct commit *commit = log->commit, *parent = log->parent;
int abbrev = opt->diffopt.abbrev;
int abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;
+ const char *extra;
int len;
opt->loginfo = NULL;
@@ -19,8 +19,17 @@ void show_log(struct rev_info *opt, stru
}
/*
- * Whitespace between commit messages, unless we are oneline
+ * The "oneline" format has several special cases:
+ * - The pretty-printed commit lacks a newline at the end
+ * of the buffer, but we do want to make sure that we
+ * have a newline there. If the separator isn't already
+ * a newline, add an extra one.
+ * - unlike other log messages, the one-line format does
+ * not have an empty line between entries.
*/
+ extra = "";
+ if (*sep != '\n' && opt->commit_format == CMIT_FMT_ONELINE)
+ extra = "\n";
if (opt->shown_one && opt->commit_format != CMIT_FMT_ONELINE)
putchar('\n');
opt->shown_one = 1;
@@ -39,7 +48,7 @@ void show_log(struct rev_info *opt, stru
* And then the pretty-printed message itself
*/
len = pretty_print_commit(opt->commit_format, commit, ~0u, this_header, sizeof(this_header), abbrev);
- printf("%s%s", this_header, sep);
+ printf("%s%s%s", this_header, extra, sep);
}
int log_tree_diff_flush(struct rev_info *opt)
^ permalink raw reply related
* Re: Log message printout cleanups
From: Junio C Hamano @ 2006-04-18 0:37 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <7vejzvka09.fsf@assigned-by-dhcp.cox.net>
Linus Torvalds <torvalds@osdl.org> writes:
> I realize that Dscho has been looking at some much "fancier" merge
> diffstats, but I really do believe that the "what got merged" difference
> to the original branch (ie parent 1) is what you want in practice.
These days I find myself running "git log --stat" more often
than "git whatchanged"; it looks so much nicer ;-).
Thanks for a job well done.
^ permalink raw reply
* Academic Qualifications available from prestigious NON-ACCREDITED universities
From: Spencer Morris @ 2006-04-18 2:28 UTC (permalink / raw)
To: git
NO ONE is turned down.
According to the U.S. Census Bureau, with the following degrees,
here's how much you can expect to make in your lifetime:
High School Diploma: $1,100,000
Bachelor's Degree: $2,100,000
Master's Degree: $2,500,000
Doctorate: $4,400,000
You Need a Better Degree, and we can Help!
Obtain degrees from Prestigious non-accredited
Universities based on you life experience.
NO ONE is turned down.
Call Now 7 days a week.
"1-718-504-5376"
^ permalink raw reply
* Re: Log message printout cleanups
From: Junio C Hamano @ 2006-04-18 0:22 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git, Johannes Schindelin
In-Reply-To: <Pine.LNX.4.64.0604171647380.3701@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> I suspect it would be better to just have the _first_
> diffstat, and always make the merge diffstat be the one for "result
> against first parent".
>
> I realize that Dscho has been looking at some much "fancier" merge
> diffstats, but I really do believe that the "what got merged" difference
> to the original branch (ie parent 1) is what you want in practice.
>
> Anyway, that's a totally separate issue, I'll let you guys fight that out.
I agree with your opinion 100% from usability point of view,
although I admit one of the fancier format was suggested by me.
Showing stat from the first parent makes --stat inconsistent
from what the "fake diff" shows, but I think it is a sensible
thing to do. The purpose of two are different in that (1) stat
is a way to see the extent of damage to what you had before you
merged, and (2) --cc is a way to see how complex the merge was,
to spot obvious merge mistakes and sanity check the merge at the
contents level.
^ permalink raw reply
* Re: Log message printout cleanups
From: Linus Torvalds @ 2006-04-17 23:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Johannes Schindelin
In-Reply-To: <7vodyzkehq.fsf@assigned-by-dhcp.cox.net>
On Mon, 17 Apr 2006, Junio C Hamano wrote:
>
> I like the new cmd_log_wc() loop very much; I was planning to do
> this myself, but I was too slow ;-).
>
> Thanks.
Here's a further patch on top of the previous one with cosmetic
improvements (no "real" code changes, just trivial updates):
- it gets the "---" before a diffstat right, including for the combined
merge case. Righ now the logic is that we always use "---" when we have
a diffstat, and an empty line otherwise. That's how I visually prefer
it, but hey, it can be tweaked later.
- I made "diff --cc/combined" add the "---/+++" header lines too. The
thing won't be mistaken for a valid diff, since the "@@" lines have too
many "@" characters (three or more), but it just makes it visually
match a real diff, which at least to me makes a big difference in
readability. Without them, it just looks very "wrong".
I guess I should have taken the filename from each individual entry
(and had one "---" file per parent), but I didn't even bother to try to
see how that works, so this was the simple thing.
With this, doing a
git log --cc --patch-with-stat
looks quite readable, I think. The only nagging issue - as far as I'm
concerned - is that diffstats for merges are pretty questionable the way
they are done now. I suspect it would be better to just have the _first_
diffstat, and always make the merge diffstat be the one for "result
against first parent".
I realize that Dscho has been looking at some much "fancier" merge
diffstats, but I really do believe that the "what got merged" difference
to the original branch (ie parent 1) is what you want in practice.
Anyway, that's a totally separate issue, I'll let you guys fight that out.
Linus
---
diff --git a/combine-diff.c b/combine-diff.c
index b4fa9c9..aef9006 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -585,6 +585,16 @@ static void reuse_combine_diff(struct sl
sline->p_lno[i] = sline->p_lno[j];
}
+static void dump_quoted_path(const char *prefix, const char *path)
+{
+ fputs(prefix, stdout);
+ if (quote_c_style(path, NULL, NULL, 0))
+ quote_c_style(path, NULL, stdout, 0);
+ else
+ printf("%s", path);
+ putchar('\n');
+}
+
static int show_patch_diff(struct combine_diff_path *elem, int num_parent,
int dense, struct rev_info *rev)
{
@@ -692,12 +702,7 @@ static int show_patch_diff(struct combin
if (rev->loginfo)
show_log(rev, rev->loginfo, "\n");
- printf("diff --%s ", dense ? "cc" : "combined");
- if (quote_c_style(elem->path, NULL, NULL, 0))
- quote_c_style(elem->path, NULL, stdout, 0);
- else
- printf("%s", elem->path);
- putchar('\n');
+ dump_quoted_path(dense ? "diff --cc " : "diff --combined ", elem->path);
printf("index ");
for (i = 0; i < num_parent; i++) {
abb = find_unique_abbrev(elem->parent[i].sha1,
@@ -728,6 +733,8 @@ static int show_patch_diff(struct combin
}
putchar('\n');
}
+ dump_quoted_path("--- a/", elem->path);
+ dump_quoted_path("+++ b/", elem->path);
dump_sline(sline, cnt, num_parent);
}
free(result);
@@ -861,6 +868,9 @@ void diff_tree_combined_merge(const unsi
&diffopts);
diffcore_std(&diffopts);
paths = intersect_paths(paths, i, num_parent);
+
+ if (diffopts.with_stat && rev->loginfo)
+ show_log(rev, rev->loginfo, "---\n");
diff_flush(&diffopts);
}
diff --git a/log-tree.c b/log-tree.c
index c0a4432..9e54164 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1,4 +1,3 @@
-
#include "cache.h"
#include "diff.h"
#include "commit.h"
@@ -55,7 +54,7 @@ int log_tree_diff_flush(struct rev_info
}
if (opt->loginfo && !opt->no_commit_id)
- show_log(opt, opt->loginfo, "\n");
+ show_log(opt, opt->loginfo, opt->diffopt.with_stat ? "---\n" : "\n");
diff_flush(&opt->diffopt);
return 1;
}
^ permalink raw reply related
* Re: Log message printout cleanups
From: Junio C Hamano @ 2006-04-17 22:45 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604171149330.3701@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> On Sun, 16 Apr 2006, Junio C Hamano wrote:
>>
>> In the mid-term, I am hoping we can drop the generate_header()
>> callchain _and_ the custom code that formats commit log in-core,
>> found in cmd_log_wc().
>
> Ok, this was nastier than expected, just because the dependencies between
> the different log-printing stuff were absolutely _everywhere_, but here's
> a patch that does exactly that.
I like the new cmd_log_wc() loop very much; I was planning to do
this myself, but I was too slow ;-).
Thanks.
^ permalink raw reply
* Re: [PATCH 1/7] cleanups: Fix resource leak and buffer overrun in daemon.c
From: Junio C Hamano @ 2006-04-17 21:42 UTC (permalink / raw)
To: Serge E. Hallyn; +Cc: git
In-Reply-To: <20060417151447.D4FE619B90E@sergelap.hallyn.com>
"Serge E. Hallyn" <serue@us.ibm.com> writes:
> Address two reports from an automatic code analyzer:
>
> 1. In logreport, it is possible to write \0 one
> character past the end of buf[].
I am perhaps slower than I usually am today, but it seems to me
that the code caps msglen to (maxlen-1) and then adds that to
buflen.
Now, maxlen is (sizeof(buf)-buflen-1), so that means after
the "buflen += msglen" happens, buflen is at most:
buflen + (sizeof(buf)-buflen-1) - 1
= sizeof(buf) - 2
And then "buf[buflen++] = '\n'; buf[buflen] = '\0'" happens.
'\n' is written at sizeof(buf)-2 (or lower index than that) and
'\0' is written at sizeof(buf)-1 (or lower). I am unsure how it
steps beyond the end...
> 2. In socksetup, socklist can be leaked when returning
> if set_reuse_addr(). Note: dunno why this case returns...
I am not sure why this part returns either. It appears to me
that it should just keep going just like the cases where
bind/listen fails.
^ permalink raw reply
* Re: [PATCH 7/7] cleanups: remove unused variable from exec_cmd.c
From: Junio C Hamano @ 2006-04-17 21:42 UTC (permalink / raw)
To: Serge E. Hallyn; +Cc: git
In-Reply-To: <20060417151448.3130F19B914@sergelap.hallyn.com>
"Serge E. Hallyn" <serue@us.ibm.com> writes:
> Not sure whether it should be removed, or whether
> execv_git_cmd() should return it rather than -1 at bottom.
The only one that does not just die() upon failure is git.c but
it checks errno itself, so I think removal is fine.
^ permalink raw reply
* Re: [PATCH 5/7] cleanups: Remove unused variable from sha1_file.c
From: Junio C Hamano @ 2006-04-17 21:42 UTC (permalink / raw)
To: Serge E. Hallyn; +Cc: git
In-Reply-To: <20060417151448.1D99C19B912@sergelap.hallyn.com>
"Serge E. Hallyn" <serue@us.ibm.com> writes:
> Left is assigned to, but never used in sha1_file.c
True, but in this case I suspect it should be used to make sure
we have the 20-byte base_sha1 that follows the header in
deltified case, perhaps like this:
diff --git a/sha1_file.c b/sha1_file.c
index e3d0113..929f481 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -874,17 +874,19 @@ void packed_object_info_detail(struct pa
unsigned char *base_sha1)
{
struct packed_git *p = e->p;
- unsigned long offset, left;
+ unsigned long offset;
unsigned char *pack;
enum object_type kind;
offset = unpack_object_header(p, e->offset, &kind, size);
pack = p->pack_base + offset;
- left = p->pack_size - offset;
if (kind != OBJ_DELTA)
*delta_chain_length = 0;
else {
unsigned int chain_length = 0;
+ if (p->pack_size - 20 < offset)
+ die("pack file %s records an incomplete delta base",
+ p->pack_name);
memcpy(base_sha1, pack, 20);
do {
struct pack_entry base_ent;
^ permalink raw reply related
* Re: [PATCH] Allow empty lines in info/grafts
From: Junio C Hamano @ 2006-04-17 21:42 UTC (permalink / raw)
To: Yann Dirson; +Cc: git
In-Reply-To: <20060417114149.28696.59020.stgit@gandelf.nowhere.earth>
Thanks; will apply.
^ permalink raw reply
* Re: [PATCH] git-rev-list: fix --header
From: Junio C Hamano @ 2006-04-17 21:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604172222390.19593@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Wouldn't this be simpler and do the same thing, I wonder? The
>> point being that "--pretty --header" and "--header --pretty"
>> traditionally did not make --header to override --pretty.
>
> I thought, why not fix that bug, too? After all, it is counterintuitive
> what "--header --pretty" does, and it was easy to fix.
I checked with 0.99.9m and both "--pretty --header" and
"--header --pretty" gives preference to --pretty. I think your
patch changes it to favor whichever comes later.
I thought it could be considered a bug to accept --header and
--pretty at the same time without complaining, but if you want
to forbid it, you could error out. However, that might break
existing Porcelains, and that's why I suggested to keep the
traditional "--pretty wins over --header" behaviour.
If gitk were the only Porcelain we care about that uses
--header, it would make more sense to change the rule to
"--header trumps --pretty" and "gitk --pretty" would magically
start working.
Still undecided. As you say it is an easy change, so I'd rather
leave the behaviour as before for now.
^ permalink raw reply
* Re: [PATCH] git-rev-list: fix --header
From: Johannes Schindelin @ 2006-04-17 20:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwtdom1t9.fsf@assigned-by-dhcp.cox.net>
Hi,
On Mon, 17 Apr 2006, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > gitk expects raw verbose headers limited by \0. Meet these expectations.
> >
> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>
> Thanks.
>
> Wouldn't this be simpler and do the same thing, I wonder? The
> point being that "--pretty --header" and "--header --pretty"
> traditionally did not make --header to override --pretty.
I thought, why not fix that bug, too? After all, it is counterintuitive
what "--header --pretty" does, and it was easy to fix.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-rev-list: fix --header
From: Junio C Hamano @ 2006-04-17 19:36 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604171443300.18017@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> gitk expects raw verbose headers limited by \0. Meet these expectations.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Thanks.
Wouldn't this be simpler and do the same thing, I wonder? The
point being that "--pretty --header" and "--header --pretty"
traditionally did not make --header to override --pretty.
diff --git a/rev-list.c b/rev-list.c
index 000f27a..d3c0dd9 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -326,6 +326,9 @@ int main(int argc, const char **argv)
else
revs.header_prefix = "commit ";
}
+ else if (revs.verbose_header)
+ /* Only --header was specified */
+ revs.commit_format = CMIT_FMT_RAW;
list = revs.commits;
^ permalink raw reply related
* Re: [PATCH] git-svnimport symlink support
From: Junio C Hamano @ 2006-04-17 19:29 UTC (permalink / raw)
To: Karl Hasselström, Martin Langhoff, Matthias Urlichs
Cc: Herbert Valerio Riedel, git
In-Reply-To: <E1FVRRH-0003Wz-6w@fencepost.gnu.org>
Herbert Valerio Riedel <hvr@gnu.org> writes:
> added svn:special symlink support for access methods other than
> direct-http
I think what the patch does makes sense. When svn:special
exists, check if it is of form "link " and munge the way a
symlink is represented to match what we expect.
My understanding is that currently there is any type of special
svn blob defined other than "link ", and dying rather than
punting and silently doing a wrong thing when we see something
else in the future makes sense to me as well.
Previously we were feeding "update-index --cacheinfo" with
'0755' and '0644', but this patch changes it to send
100755/100644; technically this is not necessary, because
create_ce_mode(m) makes it a regular file unless S_ISLNK(m), but
I think this is good for consistency.
Karl, Martin, Smurf, Comments?
> Signed-off-by: Herbert Valerio Riedel <hvr@gnu.org>
>
>
> ---
>
> git-svnimport.perl | 18 ++++++++++++++++--
> 1 files changed, 16 insertions(+), 2 deletions(-)
>
> edb2adf980e2193570a6910efc01c7ac47dcf474
> diff --git a/git-svnimport.perl b/git-svnimport.perl
> index 4d5371c..60ed7ae 100755
> --- a/git-svnimport.perl
> +++ b/git-svnimport.perl
> @@ -98,6 +98,7 @@ package SVNconn;
> use File::Spec;
> use File::Temp qw(tempfile);
> use POSIX qw(strftime dup2);
> +use Fcntl qw(SEEK_SET);
>
> sub new {
> my($what,$repo) = @_;
> @@ -143,9 +144,22 @@ sub file {
> }
> my $mode;
> if (exists $properties->{'svn:executable'}) {
> - $mode = '0755';
> + $mode = '100755';
> + } elsif (exists $properties->{'svn:special'}) {
> + my ($special_content, $filesize);
> + $filesize = tell $fh;
> + seek $fh, 0, SEEK_SET;
> + read $fh, $special_content, $filesize;
> + if ($special_content =~ s/^link //) {
> + $mode = '120000';
> + seek $fh, 0, SEEK_SET;
> + truncate $fh, 0;
> + print $fh $special_content;
> + } else {
> + die "unexpected svn:special file encountered";
> + }
> } else {
> - $mode = '0644';
> + $mode = '100644';
> }
> close ($fh);
>
> --
> 1.1.3
^ permalink raw reply
* Log message printout cleanups
From: Linus Torvalds @ 2006-04-17 18:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vbqv1oxie.fsf@assigned-by-dhcp.cox.net>
On Sun, 16 Apr 2006, Junio C Hamano wrote:
>
> In the mid-term, I am hoping we can drop the generate_header()
> callchain _and_ the custom code that formats commit log in-core,
> found in cmd_log_wc().
Ok, this was nastier than expected, just because the dependencies between
the different log-printing stuff were absolutely _everywhere_, but here's
a patch that does exactly that.
The patch is not very easy to read, and the "--patch-with-stat" thing is
still broken (it does not call the "show_log()" thing properly for
merges). That's not a new bug. In the new world order it _should_ do
something like
if (rev->logopt)
show_log(rev, rev->logopt, "---\n");
but it doesn't. I haven't looked at the --with-stat logic, so I left it
alone.
That said, this patch removes more lines than it adds, and in particular,
the "cmd_log_wc()" loop is now a very clean:
while ((commit = get_revision(rev)) != NULL) {
log_tree_commit(rev, commit);
free(commit->buffer);
commit->buffer = NULL;
}
so it doesn't get much prettier than this. All the complexity is entirely
hidden in log-tree.c, and any code that needs to flush the log literally
just needs to do the "if (rev->logopt) show_log(...)" incantation.
I had to make the combined_diff() logic take a "struct rev_info" instead
of just a "struct diff_options", but that part is pretty clean.
This does change "git whatchanged" from using "diff-tree" as the commit
descriptor to "commit", and I changed one of the tests to reflect that new
reality. Otherwise everything still passes, and my other tests look fine
too.
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
combine-diff.c | 47 ++++++--------
diff-files.c | 27 ++++----
diff-tree.c | 1
diff.h | 8 +-
git.c | 57 +----------------
log-tree.c | 167 ++++++++++++++++++++++++++++-----------------------
log-tree.h | 5 ++
rev-list.c | 9 ++-
revision.c | 3 -
revision.h | 8 +-
t/t1200-tutorial.sh | 4 +
11 files changed, 151 insertions(+), 185 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 9bd27f8..b4fa9c9 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -5,6 +5,7 @@ #include "diff.h"
#include "diffcore.h"
#include "quote.h"
#include "xdiff-interface.h"
+#include "log-tree.h"
static int uninteresting(struct diff_filepair *p)
{
@@ -585,9 +586,9 @@ static void reuse_combine_diff(struct sl
}
static int show_patch_diff(struct combine_diff_path *elem, int num_parent,
- int dense, const char *header,
- struct diff_options *opt)
+ int dense, struct rev_info *rev)
{
+ struct diff_options *opt = &rev->diffopt;
unsigned long result_size, cnt, lno;
char *result, *cp, *ep;
struct sline *sline; /* survived lines */
@@ -689,10 +690,8 @@ static int show_patch_diff(struct combin
if (show_hunks || mode_differs || working_tree_file) {
const char *abb;
- if (header) {
- shown_header++;
- printf("%s%c", header, opt->line_termination);
- }
+ if (rev->loginfo)
+ show_log(rev, rev->loginfo, "\n");
printf("diff --%s ", dense ? "cc" : "combined");
if (quote_c_style(elem->path, NULL, NULL, 0))
quote_c_style(elem->path, NULL, stdout, 0);
@@ -750,8 +749,9 @@ static int show_patch_diff(struct combin
#define COLONS "::::::::::::::::::::::::::::::::"
-static void show_raw_diff(struct combine_diff_path *p, int num_parent, const char *header, struct diff_options *opt)
+static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct rev_info *rev)
{
+ struct diff_options *opt = &rev->diffopt;
int i, offset, mod_type = 'A';
const char *prefix;
int line_termination, inter_name_termination;
@@ -761,8 +761,8 @@ static void show_raw_diff(struct combine
if (!line_termination)
inter_name_termination = 0;
- if (header)
- printf("%s%c", header, line_termination);
+ if (rev->loginfo)
+ show_log(rev, rev->loginfo, "\n");
for (i = 0; i < num_parent; i++) {
if (p->parent[i].mode)
@@ -810,31 +810,31 @@ static void show_raw_diff(struct combine
}
}
-int show_combined_diff(struct combine_diff_path *p,
+void show_combined_diff(struct combine_diff_path *p,
int num_parent,
int dense,
- const char *header,
- struct diff_options *opt)
+ struct rev_info *rev)
{
+ struct diff_options *opt = &rev->diffopt;
if (!p->len)
- return 0;
+ return;
switch (opt->output_format) {
case DIFF_FORMAT_RAW:
case DIFF_FORMAT_NAME_STATUS:
case DIFF_FORMAT_NAME:
- show_raw_diff(p, num_parent, header, opt);
- return 1;
+ show_raw_diff(p, num_parent, rev);
+ return;
default:
case DIFF_FORMAT_PATCH:
- return show_patch_diff(p, num_parent, dense, header, opt);
+ show_patch_diff(p, num_parent, dense, rev);
}
}
-const char *diff_tree_combined_merge(const unsigned char *sha1,
- const char *header, int dense,
- struct diff_options *opt)
+void diff_tree_combined_merge(const unsigned char *sha1,
+ int dense, struct rev_info *rev)
{
+ struct diff_options *opt = &rev->diffopt;
struct commit *commit = lookup_commit(sha1);
struct diff_options diffopts;
struct commit_list *parents;
@@ -874,17 +874,13 @@ const char *diff_tree_combined_merge(con
int saved_format = opt->output_format;
opt->output_format = DIFF_FORMAT_RAW;
for (p = paths; p; p = p->next) {
- if (show_combined_diff(p, num_parent, dense,
- header, opt))
- header = NULL;
+ show_combined_diff(p, num_parent, dense, rev);
}
opt->output_format = saved_format;
putchar(opt->line_termination);
}
for (p = paths; p; p = p->next) {
- if (show_combined_diff(p, num_parent, dense,
- header, opt))
- header = NULL;
+ show_combined_diff(p, num_parent, dense, rev);
}
}
@@ -894,5 +890,4 @@ const char *diff_tree_combined_merge(con
paths = paths->next;
free(tmp);
}
- return header;
}
diff --git a/diff-files.c b/diff-files.c
index 3e7f5f1..ffbef48 100644
--- a/diff-files.c
+++ b/diff-files.c
@@ -5,12 +5,14 @@
*/
#include "cache.h"
#include "diff.h"
+#include "commit.h"
+#include "revision.h"
static const char diff_files_usage[] =
"git-diff-files [-q] [-0/-1/2/3 |-c|--cc] [<common diff options>] [<path>...]"
COMMON_DIFF_OPTIONS_HELP;
-static struct diff_options diff_options;
+static struct rev_info rev;
static int silent = 0;
static int diff_unmerged_stage = 2;
static int combine_merges = 0;
@@ -18,12 +20,12 @@ static int dense_combined_merges = 0;
static void show_unmerge(const char *path)
{
- diff_unmerge(&diff_options, path);
+ diff_unmerge(&rev.diffopt, path);
}
static void show_file(int pfx, struct cache_entry *ce)
{
- diff_addremove(&diff_options, pfx, ntohl(ce->ce_mode),
+ diff_addremove(&rev.diffopt, pfx, ntohl(ce->ce_mode),
ce->sha1, ce->name, NULL);
}
@@ -31,7 +33,7 @@ static void show_modified(int oldmode, i
const unsigned char *old_sha1, const unsigned char *sha1,
char *path)
{
- diff_change(&diff_options, oldmode, mode, old_sha1, sha1, path, NULL);
+ diff_change(&rev.diffopt, oldmode, mode, old_sha1, sha1, path, NULL);
}
int main(int argc, const char **argv)
@@ -41,7 +43,7 @@ int main(int argc, const char **argv)
int entries, i;
git_config(git_diff_config);
- diff_setup(&diff_options);
+ diff_setup(&rev.diffopt);
while (1 < argc && argv[1][0] == '-') {
if (!strcmp(argv[1], "--")) {
argv++;
@@ -74,7 +76,7 @@ int main(int argc, const char **argv)
dense_combined_merges = combine_merges = 1;
else {
int diff_opt_cnt;
- diff_opt_cnt = diff_opt_parse(&diff_options,
+ diff_opt_cnt = diff_opt_parse(&rev.diffopt,
argv+1, argc-1);
if (diff_opt_cnt < 0)
usage(diff_files_usage);
@@ -89,13 +91,13 @@ int main(int argc, const char **argv)
argv++; argc--;
}
if (dense_combined_merges)
- diff_options.output_format = DIFF_FORMAT_PATCH;
+ rev.diffopt.output_format = DIFF_FORMAT_PATCH;
/* Find the directory, and set up the pathspec */
pathspec = get_pathspec(prefix, argv + 1);
entries = read_cache();
- if (diff_setup_done(&diff_options) < 0)
+ if (diff_setup_done(&rev.diffopt) < 0)
usage(diff_files_usage);
/* At this point, if argc == 1, then we are doing everything.
@@ -167,8 +169,7 @@ int main(int argc, const char **argv)
if (combine_merges && num_compare_stages == 2) {
show_combined_diff(&combine.p, 2,
dense_combined_merges,
- NULL,
- &diff_options);
+ &rev);
free(combine.p.path);
continue;
}
@@ -194,7 +195,7 @@ int main(int argc, const char **argv)
continue;
}
changed = ce_match_stat(ce, &st, 0);
- if (!changed && !diff_options.find_copies_harder)
+ if (!changed && !rev.diffopt.find_copies_harder)
continue;
oldmode = ntohl(ce->ce_mode);
@@ -207,7 +208,7 @@ int main(int argc, const char **argv)
ce->sha1, (changed ? null_sha1 : ce->sha1),
ce->name);
}
- diffcore_std(&diff_options);
- diff_flush(&diff_options);
+ diffcore_std(&rev.diffopt);
+ diff_flush(&rev.diffopt);
return 0;
}
diff --git a/diff-tree.c b/diff-tree.c
index e578798..7207867 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -71,6 +71,7 @@ int main(int argc, const char **argv)
nr_sha1 = 0;
init_revisions(opt);
opt->abbrev = 0;
+ opt->diff = 1;
argc = setup_revisions(argc, argv, opt, NULL);
while (--argc > 0) {
diff --git a/diff.h b/diff.h
index f783bae..52fff66 100644
--- a/diff.h
+++ b/diff.h
@@ -6,6 +6,7 @@ #define DIFF_H
#include "tree-walk.h"
+struct rev_info;
struct diff_options;
typedef void (*change_fn_t)(struct diff_options *options,
@@ -70,11 +71,10 @@ #define combine_diff_path_size(n, l) \
(sizeof(struct combine_diff_path) + \
sizeof(struct combine_diff_parent) * (n) + (l) + 1)
-extern int show_combined_diff(struct combine_diff_path *elem, int num_parent,
- int dense, const char *header,
- struct diff_options *);
+extern void show_combined_diff(struct combine_diff_path *elem, int num_parent,
+ int dense, struct rev_info *);
-extern const char *diff_tree_combined_merge(const unsigned char *sha1, const char *, int, struct diff_options *opt);
+extern void diff_tree_combined_merge(const unsigned char *sha1, int, struct rev_info *);
extern void diff_addremove(struct diff_options *,
int addremove,
diff --git a/git.c b/git.c
index fc4e429..016fa30 100644
--- a/git.c
+++ b/git.c
@@ -282,75 +282,22 @@ static int cmd_log_wc(int argc, const ch
struct rev_info *rev)
{
struct commit *commit;
- char *buf = xmalloc(LOGSIZE);
- const char *commit_prefix = "commit ";
- int shown = 0;
rev->abbrev = DEFAULT_ABBREV;
rev->commit_format = CMIT_FMT_DEFAULT;
+ rev->verbose_header = 1;
argc = setup_revisions(argc, argv, rev, "HEAD");
if (argc > 1)
die("unrecognized argument: %s", argv[1]);
- if (rev->commit_format == CMIT_FMT_ONELINE)
- commit_prefix = "";
prepare_revision_walk(rev);
setup_pager();
while ((commit = get_revision(rev)) != NULL) {
- unsigned long ofs = 0;
-
- if (shown && rev->diff &&
- rev->commit_format != CMIT_FMT_ONELINE)
- putchar('\n');
-
- ofs = sprintf(buf, "%s", commit_prefix);
- if (rev->abbrev_commit && rev->abbrev)
- ofs += sprintf(buf + ofs, "%s",
- find_unique_abbrev(commit->object.sha1,
- rev->abbrev));
- else
- ofs += sprintf(buf + ofs, "%s",
- sha1_to_hex(commit->object.sha1));
- if (rev->parents) {
- struct commit_list *parents = commit->parents;
- while (parents) {
- struct object *o = &(parents->item->object);
- parents = parents->next;
- if (o->flags & TMP_MARK)
- continue;
- ofs += sprintf(buf + ofs, " %s",
- sha1_to_hex(o->sha1));
- o->flags |= TMP_MARK;
- }
- /* TMP_MARK is a general purpose flag that can
- * be used locally, but the user should clean
- * things up after it is done with them.
- */
- for (parents = commit->parents;
- parents;
- parents = parents->next)
- parents->item->object.flags &= ~TMP_MARK;
- }
- buf[ofs++] =
- (rev->commit_format == CMIT_FMT_ONELINE) ? ' ' : '\n';
- ofs += pretty_print_commit(rev->commit_format, commit, ~0,
- buf + ofs,
- LOGSIZE - ofs - 20,
- rev->abbrev);
-
- if (rev->diff) {
- rev->use_precomputed_header = buf;
- strcpy(buf + ofs, "\n---\n");
- log_tree_commit(rev, commit);
- }
- else
- printf("%s\n", buf);
- shown = 1;
+ log_tree_commit(rev, commit);
free(commit->buffer);
commit->buffer = NULL;
}
- free(buf);
return 0;
}
diff --git a/log-tree.c b/log-tree.c
index af36f70..c0a4432 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -1,11 +1,51 @@
+
#include "cache.h"
#include "diff.h"
#include "commit.h"
#include "log-tree.h"
+void show_log(struct rev_info *opt, struct log_info *log, const char *sep)
+{
+ static char this_header[16384];
+ struct commit *commit = log->commit, *parent = log->parent;
+ int abbrev = opt->diffopt.abbrev;
+ int abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;
+ int len;
+
+ opt->loginfo = NULL;
+ if (!opt->verbose_header) {
+ puts(sha1_to_hex(commit->object.sha1));
+ return;
+ }
+
+ /*
+ * Whitespace between commit messages, unless we are oneline
+ */
+ if (opt->shown_one && opt->commit_format != CMIT_FMT_ONELINE)
+ putchar('\n');
+ opt->shown_one = 1;
+
+ /*
+ * Print header line of header..
+ */
+ printf("%s%s",
+ opt->commit_format == CMIT_FMT_ONELINE ? "" : "commit ",
+ diff_unique_abbrev(commit->object.sha1, abbrev_commit));
+ if (parent)
+ printf(" (from %s)", diff_unique_abbrev(parent->object.sha1, abbrev_commit));
+ putchar(opt->commit_format == CMIT_FMT_ONELINE ? ' ' : '\n');
+
+ /*
+ * And then the pretty-printed message itself
+ */
+ len = pretty_print_commit(opt->commit_format, commit, ~0u, this_header, sizeof(this_header), abbrev);
+ printf("%s%s", this_header, sep);
+}
+
int log_tree_diff_flush(struct rev_info *opt)
{
diffcore_std(&opt->diffopt);
+
if (diff_queue_is_empty()) {
int saved_fmt = opt->diffopt.output_format;
opt->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
@@ -13,12 +53,9 @@ int log_tree_diff_flush(struct rev_info
opt->diffopt.output_format = saved_fmt;
return 0;
}
- if (opt->header) {
- if (!opt->no_commit_id)
- printf("%s%c", opt->header,
- opt->diffopt.line_termination);
- opt->header = NULL;
- }
+
+ if (opt->loginfo && !opt->no_commit_id)
+ show_log(opt, opt->loginfo, "\n");
diff_flush(&opt->diffopt);
return 1;
}
@@ -43,96 +80,78 @@ static int diff_root_tree(struct rev_inf
return retval;
}
-static const char *get_header(struct rev_info *opt,
- const unsigned char *commit_sha1,
- const unsigned char *parent_sha1,
- const struct commit *commit)
-{
- static char this_header[16384];
- int offset;
- unsigned long len;
- int abbrev = opt->diffopt.abbrev;
- const char *msg = commit->buffer;
-
- if (opt->use_precomputed_header)
- return opt->use_precomputed_header;
-
- if (!opt->verbose_header)
- return sha1_to_hex(commit_sha1);
-
- len = strlen(msg);
-
- offset = sprintf(this_header, "%s%s ",
- opt->header_prefix,
- diff_unique_abbrev(commit_sha1, abbrev));
- if (commit_sha1 != parent_sha1)
- offset += sprintf(this_header + offset, "(from %s)\n",
- parent_sha1
- ? diff_unique_abbrev(parent_sha1, abbrev)
- : "root");
- else
- offset += sprintf(this_header + offset, "(from parents)\n");
- offset += pretty_print_commit(opt->commit_format, commit, len,
- this_header + offset,
- sizeof(this_header) - offset, abbrev);
- return this_header;
-}
-
-static const char *generate_header(struct rev_info *opt,
- const unsigned char *commit_sha1,
- const unsigned char *parent_sha1,
- const struct commit *commit)
-{
- const char *header = get_header(opt, commit_sha1, parent_sha1, commit);
-
- if (opt->always_show_header) {
- puts(header);
- header = NULL;
- }
- return header;
-}
-
static int do_diff_combined(struct rev_info *opt, struct commit *commit)
{
unsigned const char *sha1 = commit->object.sha1;
- opt->header = generate_header(opt, sha1, sha1, commit);
- opt->header = diff_tree_combined_merge(sha1, opt->header,
- opt->dense_combined_merges,
- &opt->diffopt);
- if (!opt->header && opt->verbose_header)
- opt->header_prefix = "\ndiff-tree ";
- return 0;
+ diff_tree_combined_merge(sha1, opt->dense_combined_merges, opt);
+ return !opt->loginfo;
}
-int log_tree_commit(struct rev_info *opt, struct commit *commit)
+/*
+ * Show the diff of a commit.
+ *
+ * Return true if we printed any log info messages
+ */
+static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log_info *log)
{
+ int showed_log;
struct commit_list *parents;
unsigned const char *sha1 = commit->object.sha1;
+ if (!opt->diff)
+ return 0;
+
/* Root commit? */
- if (opt->show_root_diff && !commit->parents) {
- opt->header = generate_header(opt, sha1, NULL, commit);
- diff_root_tree(opt, sha1, "");
+ parents = commit->parents;
+ if (!parents) {
+ if (opt->show_root_diff)
+ diff_root_tree(opt, sha1, "");
+ return !opt->loginfo;
}
/* More than one parent? */
- if (commit->parents && commit->parents->next) {
+ if (parents && parents->next) {
if (opt->ignore_merges)
return 0;
else if (opt->combine_merges)
return do_diff_combined(opt, commit);
+
+ /* If we show individual diffs, show the parent info */
+ log->parent = parents->item;
}
- for (parents = commit->parents; parents; parents = parents->next) {
+ showed_log = 0;
+ for (;;) {
struct commit *parent = parents->item;
- unsigned const char *psha1 = parent->object.sha1;
- opt->header = generate_header(opt, sha1, psha1, commit);
- diff_tree_sha1(psha1, sha1, "", &opt->diffopt);
- log_tree_diff_flush(opt);
- if (!opt->header && opt->verbose_header)
- opt->header_prefix = "\ndiff-tree ";
+ diff_tree_sha1(parent->object.sha1, sha1, "", &opt->diffopt);
+ log_tree_diff_flush(opt);
+
+ showed_log |= !opt->loginfo;
+
+ /* Set up the log info for the next parent, if any.. */
+ parents = parents->next;
+ if (!parents)
+ break;
+ log->parent = parents->item;
+ opt->loginfo = log;
+ }
+ return showed_log;
+}
+
+int log_tree_commit(struct rev_info *opt, struct commit *commit)
+{
+ struct log_info log;
+
+ log.commit = commit;
+ log.parent = NULL;
+ opt->loginfo = &log;
+
+ if (!log_tree_diff(opt, commit, &log) && opt->loginfo && opt->always_show_header) {
+ log.parent = NULL;
+ show_log(opt, opt->loginfo, "");
}
+ opt->loginfo = NULL;
return 0;
}
diff --git a/log-tree.h b/log-tree.h
index 91a909b..a26e484 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -3,9 +3,14 @@ #define LOG_TREE_H
#include "revision.h"
+struct log_info {
+ struct commit *commit, *parent;
+};
+
void init_log_tree_opt(struct rev_info *);
int log_tree_diff_flush(struct rev_info *);
int log_tree_commit(struct rev_info *, struct commit *);
int log_tree_opt_parse(struct rev_info *, const char **, int);
+void show_log(struct rev_info *opt, struct log_info *log, const char *sep);
#endif
diff --git a/rev-list.c b/rev-list.c
index b75da12..099373c 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -41,13 +41,14 @@ struct rev_info revs;
static int bisect_list = 0;
static int show_timestamp = 0;
static int hdr_termination = 0;
+static const char *header_prefix;
static void show_commit(struct commit *commit)
{
if (show_timestamp)
printf("%lu ", commit->date);
- if (*revs.header_prefix)
- fputs(revs.header_prefix, stdout);
+ if (header_prefix)
+ fputs(header_prefix, stdout);
if (commit->object.flags & BOUNDARY)
putchar('-');
if (revs.abbrev_commit && revs.abbrev)
@@ -322,9 +323,9 @@ int main(int argc, const char **argv)
/* The command line has a --pretty */
hdr_termination = '\n';
if (revs.commit_format == CMIT_FMT_ONELINE)
- revs.header_prefix = "";
+ header_prefix = "";
else
- revs.header_prefix = "commit ";
+ header_prefix = "commit ";
}
list = revs.commits;
diff --git a/revision.c b/revision.c
index 3cd6a2e..2976497 100644
--- a/revision.c
+++ b/revision.c
@@ -498,7 +498,6 @@ void init_revisions(struct rev_info *rev
revs->topo_setter = topo_sort_default_setter;
revs->topo_getter = topo_sort_default_getter;
- revs->header_prefix = "";
revs->commit_format = CMIT_FMT_DEFAULT;
diff_setup(&revs->diffopt);
@@ -675,12 +674,10 @@ int setup_revisions(int argc, const char
}
if (!strcmp(arg, "-v")) {
revs->verbose_header = 1;
- revs->header_prefix = "diff-tree ";
continue;
}
if (!strncmp(arg, "--pretty", 8)) {
revs->verbose_header = 1;
- revs->header_prefix = "diff-tree ";
revs->commit_format = get_commit_format(arg+8);
continue;
}
diff --git a/revision.h b/revision.h
index 872bcd8..48d7b4c 100644
--- a/revision.h
+++ b/revision.h
@@ -11,6 +11,7 @@ #define BOUNDARY_SHOW (1u<<6)
#define ADDED (1u<<7) /* Parents already parsed and added? */
struct rev_info;
+struct log_info;
typedef void (prune_fn_t)(struct rev_info *revs, struct commit *commit);
@@ -52,12 +53,11 @@ struct rev_info {
always_show_header:1;
/* Format info */
- unsigned int abbrev_commit:1;
+ unsigned int shown_one:1,
+ abbrev_commit:1;
unsigned int abbrev;
enum cmit_fmt commit_format;
- const char *header_prefix;
- const char *header;
- const char *use_precomputed_header;
+ struct log_info *loginfo;
/* special limits */
int max_count;
diff --git a/t/t1200-tutorial.sh b/t/t1200-tutorial.sh
index 1002413..16b3ea9 100755
--- a/t/t1200-tutorial.sh
+++ b/t/t1200-tutorial.sh
@@ -49,7 +49,7 @@ #rm hello
#test_expect_success 'git-read-tree --reset HEAD' "git-read-tree --reset HEAD ; test \"hello: needs update\" = \"$(git-update-index --refresh)\""
cat > whatchanged.expect << EOF
-diff-tree VARIABLE (from root)
+commit VARIABLE
Author: VARIABLE
Date: VARIABLE
@@ -72,7 +72,7 @@ index 0000000..557db03
EOF
git-whatchanged -p --root | \
- sed -e "1s/^\(.\{10\}\).\{40\}/\1VARIABLE/" \
+ sed -e "1s/^\(.\{7\}\).\{40\}/\1VARIABLE/" \
-e "2,3s/^\(.\{8\}\).*$/\1VARIABLE/" \
> whatchanged.output
test_expect_success 'git-whatchanged -p --root' 'cmp whatchanged.expect whatchanged.output'
^ permalink raw reply related
* [PATCH 7/7] cleanups: remove unused variable from exec_cmd.c
From: Serge E. Hallyn @ 2006-04-17 15:14 UTC (permalink / raw)
To: git
In-Reply-To: <20060417145148.2F3CB19C90D@sergelap.hallyn.com>
Not sure whether it should be removed, or whether
execv_git_cmd() should return it rather than -1 at bottom.
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
exec_cmd.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
3229a225008ab56b33b1ae7511d91f6b698cd19a
diff --git a/exec_cmd.c b/exec_cmd.c
index 590e738..44bb2f2 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -32,7 +32,7 @@ const char *git_exec_path(void)
int execv_git_cmd(const char **argv)
{
char git_command[PATH_MAX + 1];
- int len, err, i;
+ int len, i;
const char *paths[] = { current_exec_path,
getenv("GIT_EXEC_PATH"),
builtin_exec_path };
@@ -85,8 +85,6 @@ int execv_git_cmd(const char **argv)
/* execve() can only ever return if it fails */
execve(git_command, (char **)argv, environ);
- err = errno;
-
argv[0] = tmp;
}
return -1;
--
1.2.5
^ permalink raw reply related
* [PATCH 5/7] cleanups: Remove unused variable from sha1_file.c
From: Serge E. Hallyn @ 2006-04-17 15:14 UTC (permalink / raw)
To: git
In-Reply-To: <20060417145148.2F3CB19C90D@sergelap.hallyn.com>
Left is assigned to, but never used in sha1_file.c
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
sha1_file.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
0c6ecfcb112a0fb3f1ea51d8e3c220aae235a007
diff --git a/sha1_file.c b/sha1_file.c
index e3d0113..4301c80 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -874,13 +874,12 @@ void packed_object_info_detail(struct pa
unsigned char *base_sha1)
{
struct packed_git *p = e->p;
- unsigned long offset, left;
+ unsigned long offset;
unsigned char *pack;
enum object_type kind;
offset = unpack_object_header(p, e->offset, &kind, size);
pack = p->pack_base + offset;
- left = p->pack_size - offset;
if (kind != OBJ_DELTA)
*delta_chain_length = 0;
else {
--
1.2.5
^ permalink raw reply related
* [PATCH 1/7] cleanups: Fix resource leak and buffer overrun in daemon.c
From: Serge E. Hallyn @ 2006-04-17 15:14 UTC (permalink / raw)
To: git
In-Reply-To: <20060417145148.2F3CB19C90D@sergelap.hallyn.com>
Address two reports from an automatic code analyzer:
1. In logreport, it is possible to write \0 one
character past the end of buf[].
2. In socksetup, socklist can be leaked when returning
if set_reuse_addr(). Note: dunno why this case returns...
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
daemon.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
5b3e0254d34da582b7593084356c88a923f42a49
diff --git a/daemon.c b/daemon.c
index a1ccda3..7ac1bc7 100644
--- a/daemon.c
+++ b/daemon.c
@@ -65,8 +65,8 @@ static void logreport(int priority, cons
* we have space for our own LF and NUL after the "meat" of the
* message, so truncate it at maxlen - 1.
*/
- if (msglen > maxlen - 1)
- msglen = maxlen - 1;
+ if (msglen > maxlen - 2)
+ msglen = maxlen - 2;
else if (msglen < 0)
msglen = 0; /* Protect against weird return values. */
buflen += msglen;
@@ -535,6 +535,7 @@ static int socksetup(int port, int **soc
if (set_reuse_addr(sockfd)) {
close(sockfd);
+ free(socklist);
return 0; /* not fatal */
}
--
1.2.5
^ permalink raw reply related
* [PATCH 2/7] cleanups: Fix potential bugs in connect.c
From: Serge E. Hallyn @ 2006-04-17 15:14 UTC (permalink / raw)
To: git
In-Reply-To: <20060417145148.2F3CB19C90D@sergelap.hallyn.com>
The strncmp for ACK was ACK does not include the final space.
Presumably either we should either remove the trailing space,
or compare 4 chars (as this patch does).
'path' is sometimes strdup'ed, but never freed.
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
connect.c | 18 ++++++++++++++----
1 files changed, 14 insertions(+), 4 deletions(-)
1b4f45918d8c1f7d579840c7bb1fe3fe1967b6c5
diff --git a/connect.c b/connect.c
index 3f2d65c..6a8f8a6 100644
--- a/connect.c
+++ b/connect.c
@@ -74,7 +74,7 @@ int get_ack(int fd, unsigned char *resul
line[--len] = 0;
if (!strcmp(line, "NAK"))
return 0;
- if (!strncmp(line, "ACK ", 3)) {
+ if (!strncmp(line, "ACK ", 4)) {
if (!get_sha1_hex(line+4, result_sha1)) {
if (strstr(line+45, "continue"))
return 2;
@@ -567,6 +567,7 @@ int git_connect(int fd[2], char *url, co
int pipefd[2][2];
pid_t pid;
enum protocol protocol = PROTO_LOCAL;
+ int free_path = 0;
host = strstr(url, "://");
if(host) {
@@ -610,16 +611,23 @@ int git_connect(int fd[2], char *url, co
char *ptr = path;
if (path[1] == '~')
path++;
- else
+ else {
path = strdup(ptr);
+ free_path = 1;
+ }
*ptr = '\0';
}
if (protocol == PROTO_GIT) {
+ int ret;
if (git_use_proxy(host))
- return git_proxy_connect(fd, prog, host, path);
- return git_tcp_connect(fd, prog, host, path);
+ ret = git_proxy_connect(fd, prog, host, path);
+ else
+ ret = git_tcp_connect(fd, prog, host, path);
+ if (free_path)
+ free(path);
+ return ret;
}
if (pipe(pipefd[0]) < 0 || pipe(pipefd[1]) < 0)
@@ -659,6 +667,8 @@ int git_connect(int fd[2], char *url, co
fd[1] = pipefd[1][1];
close(pipefd[0][1]);
close(pipefd[1][0]);
+ if (free_path)
+ free(path);
return pid;
}
--
1.2.5
^ permalink raw reply related
* [PATCH 0/7] cleanups: intro
From: Serge E. Hallyn @ 2006-04-17 15:14 UTC (permalink / raw)
To: git
While debating whether to run git-daemon at home, I figured I'd
run some static analysis on the source. The following patches
are all pretty trivial, addressing things the tool pointed out.
thanks,
-serge
^ permalink raw reply
* [PATCH 6/7] cleanups: prevent leak of two strduped strings in config.c
From: Serge E. Hallyn @ 2006-04-17 15:14 UTC (permalink / raw)
To: git
In-Reply-To: <20060417145148.2F3CB19C90D@sergelap.hallyn.com>
Config_filename and lockfile are strduped and then leaked in
git_config_set_multivar.
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
config.c | 37 +++++++++++++++++++++++++++----------
1 files changed, 27 insertions(+), 10 deletions(-)
c96b9a1a358f8929a2525dd4e15551d286aec361
diff --git a/config.c b/config.c
index 95ec349..a37bb2d 100644
--- a/config.c
+++ b/config.c
@@ -420,6 +420,7 @@ int git_config_set_multivar(const char*
{
int i;
int fd, in_fd;
+ int ret;
char* config_filename = strdup(git_path("config"));
char* lock_file = strdup(git_path("config.lock"));
const char* last_dot = strrchr(key, '.');
@@ -431,7 +432,8 @@ int git_config_set_multivar(const char*
if (last_dot == NULL) {
fprintf(stderr, "key does not contain a section: %s\n", key);
- return 2;
+ ret = 2;
+ goto out_free;
}
store.baselen = last_dot - key;
@@ -447,7 +449,8 @@ int git_config_set_multivar(const char*
(i == store.baselen+1 && !isalpha(key[i])))) {
fprintf(stderr, "invalid key: %s\n", key);
free(store.key);
- return 1;
+ ret = 1;
+ goto out_free;
} else
store.key[i] = tolower(key[i]);
store.key[i] = 0;
@@ -460,7 +463,8 @@ int git_config_set_multivar(const char*
if (fd < 0) {
fprintf(stderr, "could not lock config file\n");
free(store.key);
- return -1;
+ ret = -1;
+ goto out_free;
}
/*
@@ -475,13 +479,15 @@ int git_config_set_multivar(const char*
strerror(errno));
close(fd);
unlink(lock_file);
- return 3; /* same as "invalid config file" */
+ ret = 3; /* same as "invalid config file" */
+ goto out_free;
}
/* if nothing to unset, error out */
if (value == NULL) {
close(fd);
unlink(lock_file);
- return 5;
+ ret = 5;
+ goto out_free;
}
store.key = (char*)key;
@@ -507,7 +513,8 @@ int git_config_set_multivar(const char*
fprintf(stderr, "Invalid pattern: %s\n",
value_regex);
free(store.value_regex);
- return 6;
+ ret = 6;
+ goto out_free;
}
}
@@ -528,7 +535,8 @@ int git_config_set_multivar(const char*
regfree(store.value_regex);
free(store.value_regex);
}
- return 3;
+ ret = 3;
+ goto out_free;
}
free(store.key);
@@ -542,7 +550,8 @@ int git_config_set_multivar(const char*
(store.seen > 1 && multi_replace == 0)) {
close(fd);
unlink(lock_file);
- return 5;
+ ret = 5;
+ goto out_free;
}
fstat(in_fd, &st);
@@ -593,10 +602,18 @@ int git_config_set_multivar(const char*
if (rename(lock_file, config_filename) < 0) {
fprintf(stderr, "Could not rename the lock file?\n");
- return 4;
+ ret = 4;
+ goto out_free;
}
- return 0;
+ ret = 0;
+
+out_free:
+ if (config_filename)
+ free(config_filename);
+ if (lock_file)
+ free(lock_file);
+ return ret;
}
--
1.2.5
^ permalink raw reply related
* [PATCH 3/7] cleanups: Remove unused vars from combine-diff.c
From: Serge E. Hallyn @ 2006-04-17 15:14 UTC (permalink / raw)
To: git
In-Reply-To: <20060417145148.2F3CB19C90D@sergelap.hallyn.com>
Mod_type in particular sure looks like it wants to be used, but isn't.
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
combine-diff.c | 12 ++----------
1 files changed, 2 insertions(+), 10 deletions(-)
5a7c4023402dd2420a4596cdd92a1a46fd3e5c14
diff --git a/combine-diff.c b/combine-diff.c
index 9bd27f8..9445e86 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -589,7 +589,7 @@ static int show_patch_diff(struct combin
struct diff_options *opt)
{
unsigned long result_size, cnt, lno;
- char *result, *cp, *ep;
+ char *result, *cp;
struct sline *sline; /* survived lines */
int mode_differs = 0;
int i, show_hunks, shown_header = 0;
@@ -641,7 +641,6 @@ static int show_patch_diff(struct combin
cnt++; /* incomplete line */
sline = xcalloc(cnt+2, sizeof(*sline));
- ep = result;
sline[0].bol = result;
for (lno = 0; lno <= cnt + 1; lno++) {
sline[lno].lost_tail = &sline[lno].lost_head;
@@ -752,7 +751,7 @@ static int show_patch_diff(struct combin
static void show_raw_diff(struct combine_diff_path *p, int num_parent, const char *header, struct diff_options *opt)
{
- int i, offset, mod_type = 'A';
+ int i, offset;
const char *prefix;
int line_termination, inter_name_termination;
@@ -764,13 +763,6 @@ static void show_raw_diff(struct combine
if (header)
printf("%s%c", header, line_termination);
- for (i = 0; i < num_parent; i++) {
- if (p->parent[i].mode)
- mod_type = 'M';
- }
- if (!p->mode)
- mod_type = 'D';
-
if (opt->output_format == DIFF_FORMAT_RAW) {
offset = strlen(COLONS) - num_parent;
if (offset < 0)
--
1.2.5
^ permalink raw reply related
* [PATCH 4/7] cleanups: Remove impossible case in quote.c
From: Serge E. Hallyn @ 2006-04-17 15:14 UTC (permalink / raw)
To: git
In-Reply-To: <20060417145148.2F3CB19C90D@sergelap.hallyn.com>
The switch is inside an if statement which is false if
the character is ' '. Either the if should be <=' '
instead of <' ', or the case should be removed as it could
be misleading.
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
quote.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
86bdfea7f2e88ed01869f370574420a6de3359d7
diff --git a/quote.c b/quote.c
index 7218a70..06792d4 100644
--- a/quote.c
+++ b/quote.c
@@ -144,8 +144,6 @@ static int quote_c_style_counted(const c
case '\\': /* fallthru */
case '"': EMITQ(); break;
- case ' ':
- break;
default:
/* octal */
EMITQ();
--
1.2.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox