* [PATCH 2/2] Fix some memory leaks in various places
From: Mike Hommey @ 2007-12-11 21:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1197407997-22945-1-git-send-email-mh@glandium.org>
Signed-off-by: Mike Hommey <mh@glandium.org>
---
builtin-init-db.c | 1 +
http-walker.c | 10 ++++++++++
walker.c | 2 ++
3 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/builtin-init-db.c b/builtin-init-db.c
index e1393b8..df61758 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -415,6 +415,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
safe_create_dir(path, 1);
strcpy(path+len, "/info");
safe_create_dir(path, 1);
+ free(path);
if (shared_repository) {
char buf[10];
diff --git a/http-walker.c b/http-walker.c
index 2c37868..1a02f86 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -231,6 +231,8 @@ static void finish_object_request(struct object_request *obj_req)
{
struct stat st;
+ free(obj_req->url);
+
fchmod(obj_req->local, 0444);
close(obj_req->local); obj_req->local = -1;
@@ -897,9 +899,17 @@ static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
static void cleanup(struct walker *walker)
{
struct walker_data *data = walker->data;
+ struct alt_base *prev_altbase, *altbase = data->alt;
+ while (altbase) {
+ free(altbase->base);
+ prev_altbase = altbase;
+ altbase = altbase->next;
+ free(prev_altbase);
+ }
http_cleanup();
curl_slist_free_all(data->no_pragma_header);
+ free(data);
}
struct walker *get_http_walker(const char *url)
diff --git a/walker.c b/walker.c
index 397b80d..7473e90 100644
--- a/walker.c
+++ b/walker.c
@@ -299,6 +299,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,
goto unlock_and_fail;
}
free(msg);
+ free(sha1);
return 0;
@@ -306,6 +307,7 @@ unlock_and_fail:
for (i = 0; i < targets; i++)
if (lock[i])
unlock_ref(lock[i]);
+ free(sha1);
return -1;
}
--
1.5.3.7
^ permalink raw reply related
* [PATCH 1/2] Fix XML parser leaks in http-push
From: Mike Hommey @ 2007-12-11 21:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
XML_Parser were never freed. While at it, move the parser initialization to
right before it is needed.
Signed-off-by: Mike Hommey <mh@glandium.org>
---
http-push.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/http-push.c b/http-push.c
index a4a9d1c..f5ba850 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1189,8 +1189,6 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
char *ep;
char timeout_header[25];
struct remote_lock *lock = NULL;
- XML_Parser parser = XML_ParserCreate(NULL);
- enum XML_Status result;
struct curl_slist *dav_headers = NULL;
struct xml_ctx ctx;
@@ -1250,6 +1248,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result == CURLE_OK) {
+ XML_Parser parser = XML_ParserCreate(NULL);
+ enum XML_Status result;
ctx.name = xcalloc(10, 1);
ctx.len = 0;
ctx.cdata = NULL;
@@ -1268,6 +1268,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
XML_GetErrorCode(parser)));
lock->timeout = -1;
}
+ XML_ParserFree(parser);
}
} else {
fprintf(stderr, "Unable to start LOCK request\n");
@@ -1428,8 +1429,6 @@ static void remote_ls(const char *path, int flags,
struct slot_results results;
struct strbuf in_buffer = STRBUF_INIT;
struct buffer out_buffer = { 0, STRBUF_INIT };
- XML_Parser parser = XML_ParserCreate(NULL);
- enum XML_Status result;
struct curl_slist *dav_headers = NULL;
struct xml_ctx ctx;
struct remote_ls_ctx ls;
@@ -1463,6 +1462,8 @@ static void remote_ls(const char *path, int flags,
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result == CURLE_OK) {
+ XML_Parser parser = XML_ParserCreate(NULL);
+ enum XML_Status result;
ctx.name = xcalloc(10, 1);
ctx.len = 0;
ctx.cdata = NULL;
@@ -1481,6 +1482,7 @@ static void remote_ls(const char *path, int flags,
XML_ErrorString(
XML_GetErrorCode(parser)));
}
+ XML_ParserFree(parser);
}
} else {
fprintf(stderr, "Unable to start PROPFIND request\n");
@@ -1512,8 +1514,6 @@ static int locking_available(void)
struct slot_results results;
struct strbuf in_buffer = STRBUF_INIT;
struct buffer out_buffer = { 0, STRBUF_INIT };
- XML_Parser parser = XML_ParserCreate(NULL);
- enum XML_Status result;
struct curl_slist *dav_headers = NULL;
struct xml_ctx ctx;
int lock_flags = 0;
@@ -1538,6 +1538,8 @@ static int locking_available(void)
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result == CURLE_OK) {
+ XML_Parser parser = XML_ParserCreate(NULL);
+ enum XML_Status result;
ctx.name = xcalloc(10, 1);
ctx.len = 0;
ctx.cdata = NULL;
@@ -1556,6 +1558,7 @@ static int locking_available(void)
XML_GetErrorCode(parser)));
lock_flags = 0;
}
+ XML_ParserFree(parser);
}
} else {
fprintf(stderr, "Unable to start PROPFIND request\n");
--
1.5.3.7
^ permalink raw reply related
* Re: git blame with valgrind massif
From: Pierre Habouzit @ 2007-12-11 21:20 UTC (permalink / raw)
To: Jon Smirl; +Cc: Git Mailing List
In-Reply-To: <9e4733910712111257h20a4a916gd4747e816e4706ff@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 584 bytes --]
On Tue, Dec 11, 2007 at 08:57:24PM +0000, Jon Smirl wrote:
> I ran:
> valgrind --tool=massif --heap=yes git blame gcc/ChangeLog
> it used about 2.25GB
>
> How do you interpret the massif output?
would you mind putting the postscript it generated somewhere too ?
it's usually pretty informative, because the amount of data allocated is
not all, its liveness is an important information too.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: git blame with valgrind massif
From: Linus Torvalds @ 2007-12-11 21:22 UTC (permalink / raw)
To: Jon Smirl; +Cc: Git Mailing List
In-Reply-To: <9e4733910712111257h20a4a916gd4747e816e4706ff@mail.gmail.com>
On Tue, 11 Dec 2007, Jon Smirl wrote:
>
> How do you interpret the massif output?
Not very easy, since massif will tell you what *allocated* it, but then
trying to see who was supposed to free it is another issue altogether.
I also find the textual output to be very confusing. But what massif is
really good at is to look at the memory usage over time in the postscript
file it generates, and that gives you a much better feel for what
particular allocation is a problem.
In this case, it's patch_delta that generates all the memory usage (well,
98% of it ;^), but that's not that helpful unless you know git internals,
and realize that with deep delta chains, that only means that the memory
is kept around just for random object data. The question is why that
object data stays around and isn't free'd.
There's two answers to that:
- the non-buggy use of the object data in the delta base cache (limited
by the delta_base_cache_limit, which defaults to 16MB, although you can
tweak it with core.deltabasecachelimit)
- the (possibly buggy) callers that keep the data.
See my previous email with a patch to "git blame" to make it release the
object data. That should fix it, I think.
Linus
^ permalink raw reply
* Re: git annotate runs out of memory
From: Daniel Berlin @ 2007-12-11 21:24 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712111122400.25032@woody.linux-foundation.org>
On 12/11/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> It's not entirely surprising that we see multiple issues with the gcc
> repo, simply because it's not the kind of repo that people have ever
> really worked on. So I don't think it's necessarily related at all, except
> in the sense of it being a different load and showing issues.
>
I'm not surprised at all.
We had a number of issues with SVN that needed to be resolved.
I'm basically trying to get issues worked (both on git and mercurial)
out to the point where it is fair for our users to try their branch
and trunk workflows with git and mercurial, and see which they like
more.
:)
^ permalink raw reply
* Re: git blame with valgrind massif
From: Pierre Habouzit @ 2007-12-11 21:27 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jon Smirl, Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712111315060.25032@woody.linux-foundation.org>
[-- Attachment #1: Type: text/plain, Size: 1211 bytes --]
On Tue, Dec 11, 2007 at 09:22:53PM +0000, Linus Torvalds wrote:
>
>
> On Tue, 11 Dec 2007, Jon Smirl wrote:
> >
> > How do you interpret the massif output?
>
> Not very easy, since massif will tell you what *allocated* it, but then
> trying to see who was supposed to free it is another issue altogether.
>
> I also find the textual output to be very confusing. But what massif is
> really good at is to look at the memory usage over time in the postscript
> file it generates, and that gives you a much better feel for what
> particular allocation is a problem.
>
> In this case, it's patch_delta that generates all the memory usage (well,
> 98% of it ;^), but that's not that helpful unless you know git internals,
No 37%, the 98% list is the list of calls that generate 98% of the
total allocations git blame does, and then massif lists each individual
call, and patch_delta generates 37% of the total.
Though it's meaningless without knowing how much time this memory
stayed allocated>
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [ANNOUNCE] ugit: a pyqt-based git gui // was: Re: If you would write git from scratch now, what would you change?
From: Jason Sewall @ 2007-12-11 21:29 UTC (permalink / raw)
To: David; +Cc: Marco Costalba, Andy Parkins, git
In-Reply-To: <402731c90712111254q1cb99c6al47538971d93b4592@mail.gmail.com>
On Dec 11, 2007 3:54 PM, David <davvid@gmail.com> wrote:
> > On Dec 11, 2007 8:14 PM, Jason Sewall <jasonsewall@gmail.com> wrote:
> >
> > I re-installed without the prefix and that error disappeared, but now I get
> > Traceback (most recent call last):
> > File "/usr/local/bin/ugit", line 12, in <module>
> > view = GitView (app.activeWindow())
> > File "../py/views.py", line 15, in __init__
> > File "default/ui/Window.py", line 43, in setupUi
> > AttributeError: setLeftMargin
>
> As for the setLeftMarginError -- That could be because you have py/qt
> 4.2. The ui files were generated with designer-qt4 (4.3.x) so you
> might need a more recent pyqt4. I'll see if I can grab an older
> version of pyqt and use it for all of the ui designs (.ui files are
> probably forward but not backwards compatible).
That was it - as a matter of fact, the package updater for my distro
was asking me to upgrade.
Well done, though I don't think I'm going to abandon git-gui quite yet.
The most valuable thing git-gui does, IMHO, is give you fine control
over what you stage in a commit. The two-paned view of staged and
unstaged changes with the view of the actual changes makes it really
easy to see exactly what you are committing. And the graphical
"{Un}stage hunk for commit" business, which ugit seems to lack so far,
is really excellent - it's much easier to use than git add -i for
partial adds.
I would like to see git add -i's hunk-splitting functionality in a
graphical tool for that matter.
Anyway, ugit is very good for a first draft; its text display beats
whats in git-gui in a big way (and I would *hope* qt4 would beat
Tcl/Tk at that at least).
One suggestion:
For Westerners like me, having "staged" on the left and "unstaged" on
the right seems a little unnatural; I'd be curious to hear others'
opinions on that.
Jason
^ permalink raw reply
* Re: git annotate runs out of memory
From: Linus Torvalds @ 2007-12-11 21:34 UTC (permalink / raw)
To: Daniel Berlin; +Cc: Git Mailing List
In-Reply-To: <4aca3dc20712111314wf4525l790120dce29a9bc5@mail.gmail.com>
On Tue, 11 Dec 2007, Daniel Berlin wrote:
>
> You theroetically can generate blame info from SVN/GIT's block deltas,
> but you of course, have the problem GIT does, which is that the delta
> is not meant to represent the actual changes that occurred, but
> instead, the smallest way to reconstruct data x from data y.
> This only sometimes has any relation to how the file actually changed
Exactly. Git objects in themselves have no history or relationships, and
being a delta against another object means nothing at all except for the
fact that the data seems to resemble that other object (which has a
_correlation_ with being related, but nothign more).
Anyway, I think the git annotate memory usage was simpyl just a real bug
that nobody had noticed before because the memory leak wasn't all that
noticeable with smaller files and/or less deep histories. Can'you verify
that it works for you with the patch I sent out?
With that fix, I could even run
git blame -C gcc/ChangeLog-2000
to see the blame machinery work past the strange "combine many different
changelogs into year-based ones" commit. Now, I cannot honestly claim that
it was really *usable* (it did take three minutes to run!), but sometimes
those three minutes of CPU time may be worth it, if it shows the real
historical context it came from.
In the case of the ChangeLog-2000 file, all the original lines obviously
came from older versions of a file called "gcc/ChangeLog", so the end
result doesn't really show what an involved situation it was to track the
sources back through not just renames, but actually file splits and
merges. Sad, but once you know what it did it's still a bit cool to see
that it worked ;)
Linus
^ permalink raw reply
* Re: git blame with valgrind massif
From: Jon Smirl @ 2007-12-11 21:45 UTC (permalink / raw)
To: Pierre Habouzit, Jon Smirl, Git Mailing List
In-Reply-To: <20071211212052.GC29110@artemis.madism.org>
On 12/11/07, Pierre Habouzit <madcoder@debian.org> wrote:
> On Tue, Dec 11, 2007 at 08:57:24PM +0000, Jon Smirl wrote:
> > I ran:
> > valgrind --tool=massif --heap=yes git blame gcc/ChangeLog
> > it used about 2.25GB
> >
> > How do you interpret the massif output?
>
> would you mind putting the postscript it generated somewhere too ?
> it's usually pretty informative, because the amount of data allocated is
> not all, its liveness is an important information too.
It was very boring. A diagonal line from 0 to 2GB.
>
> --
> ·O· Pierre Habouzit
> ··O madcoder@debian.org
> OOO http://www.madism.org
>
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: git annotate runs out of memory
From: Junio C Hamano @ 2007-12-11 21:54 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Daniel Berlin, Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712111300440.25032@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> builtin-blame.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-blame.c b/builtin-blame.c
> index c158d31..18f9924 100644
> --- a/builtin-blame.c
> +++ b/builtin-blame.c
> @@ -87,6 +87,14 @@ struct origin {
> char path[FLEX_ARRAY];
> };
>
> /*
> * Given an origin, prepare mmfile_t structure to be used by the
> * diff machinery
> @@ -558,6 +566,8 @@ static struct patch *get_patch(struct origin *parent, struct origin *origin)
> if (!file_p.ptr || !file_o.ptr)
> return NULL;
> patch = compare_buffer(&file_p, &file_o, 0);
> + drop_origin_blob(parent);
> + drop_origin_blob(origin);
> num_get_patch++;
> return patch;
> }
While this should be safe (because the user of blob lazily re-fetches),
it feels a bit too aggressive, especially when -C or other "retry and
try harder to assign blame elsewhere" option is used.
Instead, how about discarding after we are done with each origin, like
this?
---
builtin-blame.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index c158d31..eda79d0 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -130,6 +130,14 @@ static void origin_decref(struct origin *o)
}
}
+static void drop_origin_blob(struct origin *o)
+{
+ if (o->file.ptr) {
+ free(o->file.ptr);
+ o->file.ptr = NULL;
+ }
+}
+
/*
* Each group of lines is described by a blame_entry; it can be split
* as we pass blame to the parents. They form a linked list in the
@@ -1274,8 +1282,13 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
}
finish:
- for (i = 0; i < MAXPARENT; i++)
- origin_decref(parent_origin[i]);
+ for (i = 0; i < MAXPARENT; i++) {
+ if (parent_origin[i]) {
+ drop_origin_blob(parent_origin[i]);
+ origin_decref(parent_origin[i]);
+ }
+ }
+ drop_origin_blob(origin);
}
/*
^ permalink raw reply related
* [PATCH 3/2] Fix small memory leaks induced by diff_tree_setup_paths
From: Mike Hommey @ 2007-12-11 21:59 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1197407997-22945-2-git-send-email-mh@glandium.org>
Run diff_tree_release_paths in the appropriate places, and add a test to
avoid NULL dereference. Better safe than sorry.
Signed-off-by: Mike Hommey <mh@glandium.org>
---
There are a few more occurrences of diff_tree_setup_paths for which I don't
know the code enough to correctly place the corresponding
diff_tree_release_paths. Interesting the function existed but was never
actually used ;)
builtin-blame.c | 4 +++-
builtin-reset.c | 1 +
tree-diff.c | 4 +++-
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index 523e55a..041901b 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -388,6 +388,7 @@ static struct origin *find_origin(struct scoreboard *sb,
}
}
diff_flush(&diff_opts);
+ diff_tree_release_paths(&diff_opts);
if (porigin) {
/*
* Create a freestanding copy that is not part of
@@ -444,6 +445,7 @@ static struct origin *find_rename(struct scoreboard *sb,
}
}
diff_flush(&diff_opts);
+ diff_tree_release_paths(&diff_opts);
return porigin;
}
@@ -1167,7 +1169,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
}
}
diff_flush(&diff_opts);
-
+ diff_tree_release_paths(&diff_opts);
return retval;
}
diff --git a/builtin-reset.c b/builtin-reset.c
index 4c61025..713c2d5 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -158,6 +158,7 @@ static int read_from_tree(const char *prefix, const char **argv,
return 1;
diffcore_std(&opt);
diff_flush(&opt);
+ diff_tree_release_paths(&opt);
if (!index_was_discarded)
/* The index is still clobbered from do_diff_cache() */
diff --git a/tree-diff.c b/tree-diff.c
index aa0a100..a2fdf86 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -326,6 +326,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
die("unable to set up diff options to follow renames");
diff_tree(t1, t2, base, &diff_opts);
diffcore_std(&diff_opts);
+ diff_tree_release_paths(&diff_opts);
/* Go through the new set of filepairing, and see if we find a more interesting one */
for (i = 0; i < q->nr; i++) {
@@ -418,7 +419,8 @@ static int count_paths(const char **paths)
void diff_tree_release_paths(struct diff_options *opt)
{
- free(opt->pathlens);
+ if (opt->pathlens)
+ free(opt->pathlens);
}
void diff_tree_setup_paths(const char **p, struct diff_options *opt)
--
1.5.3.7
^ permalink raw reply related
* [PATCH] Fix git-fast-export for zero-sized blobs
From: Alex Riesen @ 2007-12-11 22:01 UTC (permalink / raw)
To: git; +Cc: Nicolas Pitre, Johannes Schindelin, Junio C Hamano
In-Reply-To: <alpine.LFD.0.99999.0712111509270.555@xanadu.home>
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
builtin-fast-export.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
Nicolas Pitre, Tue, Dec 11, 2007 21:27:33 +0100:
> Simply doing something like:
>
> $ git fast-export --all > /dev/null
>
> results in:
>
> fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
>
I get this for my git repo:
$ git fast-export --all >/dev/null
fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
$ git cat-file blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 |wc -c
0
Writing a zero-size blob will surely returns 0.
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 2136aad..ef27eee 100755
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -103,7 +103,7 @@ static void handle_object(const unsigned char *sha1)
mark_object(object);
printf("blob\nmark :%d\ndata %lu\n", last_idnum, size);
- if (fwrite(buf, size, 1, stdout) != 1)
+ if (size && fwrite(buf, size, 1, stdout) != 1)
die ("Could not write blob %s", sha1_to_hex(sha1));
printf("\n");
--
1.5.3.7.1177.gb22b
^ permalink raw reply related
* Re: 'git fast-export' is crashing on the gcc repo
From: Nicolas Pitre @ 2007-12-11 22:06 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.LFD.0.99999.0712111509270.555@xanadu.home>
On Tue, 11 Dec 2007, Nicolas Pitre wrote:
> Simply doing something like:
>
> $ git fast-export --all > /dev/null
>
> results in:
>
> fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
>
> Since this is extremely enlightening, I patched it as follows:
>
> diff --git a/builtin-fast-export.c b/builtin-fast-export.c
> index 2136aad..5c7bfe0 100755
> --- a/builtin-fast-export.c
> +++ b/builtin-fast-export.c
> @@ -104,7 +104,8 @@ static void handle_object(const unsigned char *sha1)
>
> printf("blob\nmark :%d\ndata %lu\n", last_idnum, size);
> if (fwrite(buf, size, 1, stdout) != 1)
> - die ("Could not write blob %s", sha1_to_hex(sha1));
> + die ("Could not write blob %s: %s",
> + sha1_to_hex(sha1), strerror(errno));
> printf("\n");
>
> show_progress();
>
> And then running it again produced:
>
> fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391: Inappropriate ioctl for device
>
> adding to today's confusion.
Well, ignore the above. It seems that most of stdio doesn't set errno
so the above is crap.
Even strace doesn't show any error.
But, somehow, the following patch fixes it:
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 2136aad..c32a124 100755
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -103,7 +103,7 @@ static void handle_object(const unsigned char *sha1)
mark_object(object);
printf("blob\nmark :%d\ndata %lu\n", last_idnum, size);
- if (fwrite(buf, size, 1, stdout) != 1)
+ if (fwrite(buf, 1, size, stdout) != size)
die ("Could not write blob %s", sha1_to_hex(sha1));
printf("\n");
Go figure.
Nicolas
^ permalink raw reply related
* Re: [PATCH] Fix git-fast-export for zero-sized blobs
From: Nicolas Pitre @ 2007-12-11 22:20 UTC (permalink / raw)
To: Alex Riesen; +Cc: git, Johannes Schindelin, Junio C Hamano
In-Reply-To: <20071211220128.GA19857@steel.home>
On Tue, 11 Dec 2007, Alex Riesen wrote:
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---
> builtin-fast-export.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> Nicolas Pitre, Tue, Dec 11, 2007 21:27:33 +0100:
> > Simply doing something like:
> >
> > $ git fast-export --all > /dev/null
> >
> > results in:
> >
> > fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> >
>
> I get this for my git repo:
>
> $ git fast-export --all >/dev/null
> fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> $ git cat-file blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 |wc -c
> 0
>
> Writing a zero-size blob will surely returns 0.
Indeed.
/me looking at too many piece of code at the same time
Nicolas
^ permalink raw reply
* Re: [PATCH] Invert numbers and names in the git-shortlog summary mode.
From: Junio C Hamano @ 2007-12-11 22:21 UTC (permalink / raw)
To: Ingo Molnar
Cc: Pierre Habouzit, Jeff King, Jakub Narebski, Christian Couder, git
In-Reply-To: <20071211211341.GB6902@elte.hu>
Ingo Molnar <mingo@elte.hu> writes:
> * Pierre Habouzit <madcoder@debian.org> wrote:
>
>> The next git has a _lot_ of things done better wrt UI and such issues.
>> Though some backward incompatible changes must be introduced with the
>> proper deprecation warnings, so that people can adapt.
>
> hey, cool! Just when i decide to complain about it, after 2 years of
> suffering, it's already fixed in the devel branch =;-)
Heh, as I saw you post "diff --git" patches to the kernel mailing list
and heard nothing from you for the last two years here, I naturally
assumed you were one of the many people who were perfectly happy with
git.
Please direct more complaints to this list, not to the kernel list nor
elsewhere ;-).
^ permalink raw reply
* Re: git-bisect feature suggestion: "git-bisect diff"
From: しらいしななこ @ 2007-12-11 22:22 UTC (permalink / raw)
To: Jeff King
Cc: Pierre Habouzit, Jakub Narebski, Ingo Molnar, Christian Couder,
git, Junio C Hamano
Quoting Jeff King <peff@peff.net>:
> On Tue, Dec 11, 2007 at 12:59:14PM +0100, Pierre Habouzit wrote:
>
>> > Not exactly, as it does not give us email address.
>>
>> maybe it should be "fixed" so that it does, not to mention that other
>> concerns ingo raised look legit to me.
>
> Perhaps Junio is a time-traveller.
>
> $ git show 4602c17d
> commit 4602c17d8911e14d537f6f87db02faab6e3f5d69
> Author: Junio C Hamano <gitster@pobox.com>
> Date: Fri Dec 7 17:19:31 2007 -0800
>
> git-shortlog -e: show e-mail address as well
>
> This option shows the author's email address next to the name.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Or, Junio and Ingo are the same person ;-)
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
----------------------------------------------------------------------
Free pop3 email with a spam filter.
http://www.bluebottle.com/tag/5
^ permalink raw reply
* [PATCH 1/2 v2] Fix XML parser leaks in http-push
From: Mike Hommey @ 2007-12-11 22:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1197407997-22945-1-git-send-email-mh@glandium.org>
XML_Parser were never freed. While at it, move the parser initialization to
right before it is needed.
Signed-off-by: Mike Hommey <mh@glandium.org>
---
This one is the same, but against pu, where Junio fixed my strbuf patch in
a different way than I did.
http-push.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/http-push.c b/http-push.c
index a76a9e5..c7586b3 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1189,8 +1189,6 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
char *ep;
char timeout_header[25];
struct remote_lock *lock = NULL;
- XML_Parser parser = XML_ParserCreate(NULL);
- enum XML_Status result;
struct curl_slist *dav_headers = NULL;
struct xml_ctx ctx;
@@ -1250,6 +1248,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result == CURLE_OK) {
+ XML_Parser parser = XML_ParserCreate(NULL);
+ enum XML_Status result;
ctx.name = xcalloc(10, 1);
ctx.len = 0;
ctx.cdata = NULL;
@@ -1268,6 +1268,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
XML_GetErrorCode(parser)));
lock->timeout = -1;
}
+ XML_ParserFree(parser);
}
} else {
fprintf(stderr, "Unable to start LOCK request\n");
@@ -1428,8 +1429,6 @@ static void remote_ls(const char *path, int flags,
struct slot_results results;
struct strbuf in_buffer = STRBUF_INIT;
struct buffer out_buffer = { STRBUF_INIT, 0 };
- XML_Parser parser = XML_ParserCreate(NULL);
- enum XML_Status result;
struct curl_slist *dav_headers = NULL;
struct xml_ctx ctx;
struct remote_ls_ctx ls;
@@ -1463,6 +1462,8 @@ static void remote_ls(const char *path, int flags,
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result == CURLE_OK) {
+ XML_Parser parser = XML_ParserCreate(NULL);
+ enum XML_Status result;
ctx.name = xcalloc(10, 1);
ctx.len = 0;
ctx.cdata = NULL;
@@ -1481,6 +1482,7 @@ static void remote_ls(const char *path, int flags,
XML_ErrorString(
XML_GetErrorCode(parser)));
}
+ XML_ParserFree(parser);
}
} else {
fprintf(stderr, "Unable to start PROPFIND request\n");
@@ -1512,8 +1514,6 @@ static int locking_available(void)
struct slot_results results;
struct strbuf in_buffer = STRBUF_INIT;
struct buffer out_buffer = { STRBUF_INIT, 0 };
- XML_Parser parser = XML_ParserCreate(NULL);
- enum XML_Status result;
struct curl_slist *dav_headers = NULL;
struct xml_ctx ctx;
int lock_flags = 0;
@@ -1538,6 +1538,8 @@ static int locking_available(void)
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result == CURLE_OK) {
+ XML_Parser parser = XML_ParserCreate(NULL);
+ enum XML_Status result;
ctx.name = xcalloc(10, 1);
ctx.len = 0;
ctx.cdata = NULL;
@@ -1556,6 +1558,7 @@ static int locking_available(void)
XML_GetErrorCode(parser)));
lock_flags = 0;
}
+ XML_ParserFree(parser);
}
} else {
fprintf(stderr, "Unable to start PROPFIND request\n");
--
1.5.3.7.1164.ga23bb-dirty
^ permalink raw reply related
* Re: [PATCH 1/2 v2] Fix XML parser leaks in http-push
From: Junio C Hamano @ 2007-12-11 22:36 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <1197412253-927-1-git-send-email-mh@glandium.org>
Mike Hommey <mh@glandium.org> writes:
> XML_Parser were never freed. While at it, move the parser initialization to
> right before it is needed.
>
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>
> This one is the same, but against pu, where Junio fixed my strbuf patch in
> a different way than I did.
To be very honest, I wish the strbuf patch (that has potentially larger
impact) did not take these obviously correct leakfix patches hostage.
Will have to take a look but slowly (I'm at work now).
^ permalink raw reply
* Re: [ANNOUNCE] ugit: a pyqt-based git gui // was: Re: If you would write git from scratch now, what would you change?
From: Alex Riesen @ 2007-12-11 22:37 UTC (permalink / raw)
To: David; +Cc: Andy Parkins, git
In-Reply-To: <402731c90712110548k67f28b64w5afa93ee908ce73b@mail.gmail.com>
David, Tue, Dec 11, 2007 14:48:32 +0100:
> Though there's still a few things remaining to be implemented, the
> bulk of the initial groundwork is already done. All you need to
> build/run it is python and pyqt4 (pyuic4). I've deliberately tried to
> keep the interface similar to git-gui for now since it is obviously
> based on it, but that's not a requirement.
Interesting. I had to start it like this:
$ export PYTHONPATH=$(pwd)/build/default:$(pwd)/build/default/ui
$ python ./build/default/bin/ugit.pyc
It has some problem with merges in "Git Commit Browser": takes a lot
of CPU and very slowly generates a very big diff.
The diff view is very ... dark. Out of place, when the rest of the
interface corresponds to system theme (mine is rather light).
^ permalink raw reply
* [PATCH 1/2 for master] Fix XML parser leaks in http-push
From: Mike Hommey @ 2007-12-11 22:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <7v4peodfkb.fsf@gitster.siamese.dyndns.org>
XML_Parser were never freed. While at it, move the parser initialization to
right before it is needed.
Signed-off-by: Mike Hommey <mh@glandium.org>
---
Same, on top of master.
http-push.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/http-push.c b/http-push.c
index 78283b4..fffbe9c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1275,8 +1275,6 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
char *ep;
char timeout_header[25];
struct remote_lock *lock = NULL;
- XML_Parser parser = XML_ParserCreate(NULL);
- enum XML_Status result;
struct curl_slist *dav_headers = NULL;
struct xml_ctx ctx;
@@ -1345,6 +1343,8 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result == CURLE_OK) {
+ XML_Parser parser = XML_ParserCreate(NULL);
+ enum XML_Status result;
ctx.name = xcalloc(10, 1);
ctx.len = 0;
ctx.cdata = NULL;
@@ -1363,6 +1363,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
XML_GetErrorCode(parser)));
lock->timeout = -1;
}
+ XML_ParserFree(parser);
}
} else {
fprintf(stderr, "Unable to start LOCK request\n");
@@ -1525,8 +1526,6 @@ static void remote_ls(const char *path, int flags,
struct buffer out_buffer;
char *in_data;
char *out_data;
- XML_Parser parser = XML_ParserCreate(NULL);
- enum XML_Status result;
struct curl_slist *dav_headers = NULL;
struct xml_ctx ctx;
struct remote_ls_ctx ls;
@@ -1569,6 +1568,8 @@ static void remote_ls(const char *path, int flags,
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result == CURLE_OK) {
+ XML_Parser parser = XML_ParserCreate(NULL);
+ enum XML_Status result;
ctx.name = xcalloc(10, 1);
ctx.len = 0;
ctx.cdata = NULL;
@@ -1587,6 +1588,7 @@ static void remote_ls(const char *path, int flags,
XML_ErrorString(
XML_GetErrorCode(parser)));
}
+ XML_ParserFree(parser);
}
} else {
fprintf(stderr, "Unable to start PROPFIND request\n");
@@ -1620,8 +1622,6 @@ static int locking_available(void)
struct buffer out_buffer;
char *in_data;
char *out_data;
- XML_Parser parser = XML_ParserCreate(NULL);
- enum XML_Status result;
struct curl_slist *dav_headers = NULL;
struct xml_ctx ctx;
int lock_flags = 0;
@@ -1658,6 +1658,8 @@ static int locking_available(void)
if (start_active_slot(slot)) {
run_active_slot(slot);
if (results.curl_result == CURLE_OK) {
+ XML_Parser parser = XML_ParserCreate(NULL);
+ enum XML_Status result;
ctx.name = xcalloc(10, 1);
ctx.len = 0;
ctx.cdata = NULL;
@@ -1676,6 +1678,7 @@ static int locking_available(void)
XML_GetErrorCode(parser)));
lock_flags = 0;
}
+ XML_ParserFree(parser);
}
} else {
fprintf(stderr, "Unable to start PROPFIND request\n");
--
1.5.3.7.1164.ga23bb-dirty
^ permalink raw reply related
* Re: [ANNOUNCE] ugit: a pyqt-based git gui // was: Re: If you would write git from scratch now, what would you change?
From: Steffen Prohaska @ 2007-12-11 23:08 UTC (permalink / raw)
To: David; +Cc: Andy Parkins, Git Mailing List, Alex Riesen
In-Reply-To: <20071211223742.GB19857@steel.home>
On Dec 11, 2007, at 11:37 PM, Alex Riesen wrote:
> David, Tue, Dec 11, 2007 14:48:32 +0100:
>> Though there's still a few things remaining to be implemented, the
>> bulk of the initial groundwork is already done. All you need to
>> build/run it is python and pyqt4 (pyuic4). I've deliberately
>> tried to
>> keep the interface similar to git-gui for now since it is obviously
>> based on it, but that's not a requirement.
>
> Interesting. I had to start it like this:
>
> $ export PYTHONPATH=$(pwd)/build/default:$(pwd)/build/default/ui
> $ python ./build/default/bin/ugit.pyc
>
> It has some problem with merges in "Git Commit Browser": takes a lot
> of CPU and very slowly generates a very big diff.
>
> The diff view is very ... dark. Out of place, when the rest of the
> interface corresponds to system theme (mine is rather light).
Yeah, it's dark here (Mac OS X), too -- doesn't look very
friendly.
It took me some time to get the full Qt, SIP, PyQT toolchain
running ... below's my first (tiny) fix that makes
"Branch > Create ..." work if you don't select track.
Steffen
diff --git a/py/cmds.py b/py/cmds.py
index 5abf930..4a35ead 100644
--- a/py/cmds.py
+++ b/py/cmds.py
@@ -119,8 +119,8 @@ def git_create_branch (name, base, track=False):
'''Creates a branch starting from base. Pass track=True
to create a remote tracking branch.'''
cmd = 'git branch'
- if track: cmd += ' --track '
- cmd += '%s %s' % ( utils.shell_quote (name),
+ if track: cmd += ' --track'
+ cmd += ' %s %s' % ( utils.shell_quote (name),
utils.shell_quote (base))
return commands.getoutput (cmd)
^ permalink raw reply related
* Re: [PATCH (amend)] diff: Make numstat machine friendly also for renames (and copies)
From: Jakub Narebski @ 2007-12-11 23:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vprxehrlv.fsf@gitster.siamese.dyndns.org>
On Wed, 11 Dec 2007, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> Junio C Hamano wrote:
>>> Jakub Narebski <jnareb@gmail.com> writes:
>>>
>>>> Previous version of this patch (from 7 May 2007) used instead of current
>>>> only "to_name" format similar to git-diff-tree raw format for renames:
>>>>
>>>> added deleted TAB path for "src" TAB path for "dst" LF
>
> That's perfectly fine (that is without -z).
>
>>>> ... Previous version
>>>> of patch used (or was to use actually, because of error in the code)
>>>>
>>>> added deleted TAB path for "src" NUL NUL path for "dst" NUL
>>>>
>>>> when -z option was used.
>
> I think your "previous" one:
>
> <added> <deleted> <src> NUL (no rename)
> <added> <deleted> <src> NUL NUL <dst> NUL (with rename)
>
> would be unambiguously parsable, but it would be cleaner and easier for
> the parser if the latter were like this instead:
>
> <added> <deleted> NUL <src> NUL <dst> NUL (with rename)
>
> The reader can expect to find a NUL terminated src, finds the length is
> zero and notices it cannot be src and then reads two paths from that
> point.
Very good idea.
> Without having the status letter, we cannot do "optional two paths"
> without breaking existing --numstat -z readers that do not care about
> renames and copies, and I agree that existing --numstat -z readers that
> pass -M or -C are already broken, so I think a format extension along
> the above line (your "previous" and the above clean-up proposal have the
> same power of expressiveness, I just think the latter is syntactically
> cleaner) would be reasonable. And at that point we probably would not
> need --numstat-enhanced at all ;-)
The previous patch is a fix (usability fix) for numstat output in the
presence of rename detection, making it truly machine friendly. Moreover
it should not break any scripts which parsed numstat output not
expecting to deal with renames (for example if repository has
diff.renames config option set), and might even fix them.
The proposed - and implemented in patch below - change could break some
scripts not expecting to deal with new numstat output. Adding status
letter (with similarity/dissimilarity percentage info) would
unfortunately require greater surgery... We can either allow scripts
(do there exist any?) to break, or make the full rename info shown
only for --numstat-extended; but I'd like to have this status letter
for --numstat-extended / --numstat-enhanced.
So possible solution are:
(a) Accept both patches, and accept remote possibility that some
scripts might break
(b) Add --numstat-extended option whoich would show rename info
as implemented in this patch (and of course accept previous)
(c) Implement --numstat-extended with status letter, as suggested,
accept first patch but not this one.
Note that gitweb would require --numstat with proper rename detection
support if we want to e.g. provide graphical diffstat in 'commit' view
for merge commits.
P.S. The numstat output format for renames should be probably described
in documentation, and not only in commit message, but I was not sure
where to put it (and even how it should be written).
-- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Date: Tue, 11 Dec 2007 23:52:18 +0100
Subject: [RFC/PATCH] diff: Show rename info in numstat output, correctly
Make numstat output of git-diff show both source and destination
filename for renames (and copies) in easily parseable format, similar
to raw diff output format.
The numstat format for rename (or copy) is now
<added> <deleted> TAB <path for "src"> TAB <path for "dst"> LF
or if -z option is used
<added> <deleted> TAB NULL <path for "src"> NULL <path for "dst"> NULL
where <added> and <deleted> is number of added/deleted lines
(or '-' for binary file).
When -z option is not used, TAB, LF, and backslash characters in
pathnames are represented as \t, \n, and \\, respectively.
Idea for -z numstat rename format by Junio C Hamano.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
diff.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/diff.c b/diff.c
index 8039ac7..56cbf28 100644
--- a/diff.c
+++ b/diff.c
@@ -991,7 +991,14 @@ static void show_numstat(struct diffstat_t* data, struct diff_options *options)
printf("-\t-\t");
else
printf("%d\t%d\t", file->added, file->deleted);
- write_name_quoted(file->name, stdout, options->line_termination);
+ if (file->is_renamed) {
+ char sep = options->line_termination ? '\t' : '\0';
+ if (!options->line_termination)
+ putchar(options->line_termination);
+ write_name_quoted(file->from_name, stdout, sep);
+ write_name_quoted(file->name, stdout, options->line_termination);
+ } else
+ write_name_quoted(file->name, stdout, options->line_termination);
}
}
--
1.5.3.7
^ permalink raw reply related
* Re: gitk graph routing problem
From: Paul Mackerras @ 2007-12-11 22:26 UTC (permalink / raw)
To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20071104104618.GA3078@steel.home>
Alex Riesen writes:
> To reproduce, try running in git repo:
>
> gitk 02f630448e5d48e..06ea6ba9cf46ef5
>
> Than go some pages (around 5) forward. You should notice system load
> going up rapidly. Now try paging back - and graph starts stretching
> to the right, to the point nothing fits on the screen anymore.
I finally got back to look at this. The problem is not so much the
layout algorithm per se as the fact that I haven't worked out a good
way to pack lots of downward-pointing arrows in without using up
arbitrarily large amounts of horizontal space. You have managed to
find an example where just about every commit is a merge needing one
or more downward-pointing arrows.
Incidentally, gitk from the dev branch of my gitk.git repo does much
better on this example, since it is able to hoist the open-circle
(excluded) commits up to the row below their merge children, which
looks much nicer.
Paul.
^ permalink raw reply
* Re: [PATCH 4/9] gitk i18n: Initial German translation.
From: Paul Mackerras @ 2007-12-11 23:22 UTC (permalink / raw)
To: Christian Stimming; +Cc: Junio C Hamano, git
In-Reply-To: <200711071844.03106.stimming@tuhh.de>
Christian Stimming writes:
> ---
> In order to test the i18n code, I prepared half-completed
> German translation.
>
> po/de.po | 703 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 703 insertions(+), 0 deletions(-)
> create mode 100644 po/de.po
>
> diff --git a/po/de.po b/po/de.po
> new file mode 100644
> index 0000000..8c41069
> --- /dev/null
> +++ b/po/de.po
> @@ -0,0 +1,703 @@
> +# Translation of git-gui to German.
> +# Copyright (C) 2007 Shawn Pearce, et al.
Ummm? I think you forgot to change some things here. :)
Paul.
^ permalink raw reply
* Re: git annotate runs out of memory
From: Linus Torvalds @ 2007-12-11 23:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Daniel Berlin, Git Mailing List
In-Reply-To: <7vprxcdhis.fsf@gitster.siamese.dyndns.org>
On Tue, 11 Dec 2007, Junio C Hamano wrote:
>
> Instead, how about discarding after we are done with each origin, like
> this?
Sure, looks fine to me. With either of these patches, all of the cost is
in the diffing routines:
samples % image name app name symbol name
191317 31.4074 git git xdl_hash_record
120060 19.7096 git git xdl_recmatch
99286 16.2992 git git xdl_prepare_ctx
56370 9.2539 libc-2.7.so libc-2.7.so memcpy
23315 3.8275 git git xdl_prepare_env
..
and while I suspect xdiff could be optimized a bit more for the cases
where we have no changes at the end, that's beyond my skills.
Linus
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox