* Re: [PATCH] diff-delta.c: rename {a,}{entry,hash} to {,u}{entry,hash}
From: Nicolas Pitre @ 2007-09-08 20:52 UTC (permalink / raw)
To: David Kastrup; +Cc: git
In-Reply-To: <f1161c08aeeca60b6c33af34ccea68fd99b9ea4e.1189243702.git.dak@gnu.org>
On Sat, 8 Sep 2007, David Kastrup wrote:
> The variables for the packed entries are now called just entry and
> hash rather than aentry+ahash, and those for the unpacked entries have
> been renamed to uentry and uhash from the original entry and hash.
>
> While this makes the diff to the unchanged code larger, it matches the
> type declarations better.
Since the packed version is used less often, could you rename that one
instead, and use packed_entry and packed_hash forclarity.
Also please fold this with your other patch (not the culling one).
Then you can add my ACK.
Nicolas
^ permalink raw reply
* Re: [PATCH] diff-delta.c: rename {a,}{entry,hash} to {,u}{entry,hash}
From: David Kastrup @ 2007-09-08 21:06 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.0.9999.0709081648281.21186@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> On Sat, 8 Sep 2007, David Kastrup wrote:
>
>> The variables for the packed entries are now called just entry and
>> hash rather than aentry+ahash, and those for the unpacked entries have
>> been renamed to uentry and uhash from the original entry and hash.
>>
>> While this makes the diff to the unchanged code larger, it matches the
>> type declarations better.
>
> Since the packed version is used less often, could you rename that one
> instead, and use packed_entry and packed_hash forclarity.
>
> Also please fold this with your other patch (not the culling one).
Uh, what other patch? If I don't do the renames in the second patch,
then only the first patch remains. Which I'll change to use the
proposed names. Hope that's what you meant.
> Then you can add my ACK.
I'll do that, and rework the culling patch on top of that (unacked for
now).
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* [PATCH] diff-delta.c: pack the index structure
From: David Kastrup @ 2007-09-08 21:17 UTC (permalink / raw)
To: git
In normal use cases, the performance wins are not overly impressive:
we get something like 5-10% due to the slightly better locality of
memory accesses using the packed structure.
However, since the data structure for index entries saves 33% of
memory on 32-bit platforms and 40% on 64-bit platforms, the behavior
when memory gets limited should be nicer.
This is a rather well-contained change. One obvious improvement would
be sorting the elements in one bucket according to their hash, then
using binary probing to find the elements with the right hash value.
As it stands, the output should be strictly the same as previously
unless one uses the option for limiting the amount of used memory, in
which case the created packs might be better.
Signed-off-by: David Kastrup <dak@gnu.org>
Acked-by: Nicolas Pitre <nico@cam.org>
---
diff-delta.c | 74 +++++++++++++++++++++++++++++++++++++++++++++------------
1 files changed, 58 insertions(+), 16 deletions(-)
diff --git a/diff-delta.c b/diff-delta.c
index 0dde2f2..d355e5e 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -115,9 +115,13 @@ static const unsigned int U[256] = {
struct index_entry {
const unsigned char *ptr;
unsigned int val;
- struct index_entry *next;
};
+struct unpacked_index_entry {
+ struct index_entry entry;
+ struct unpacked_index_entry *next;
+};
+
struct delta_index {
unsigned long memsize;
const void *src_buf;
@@ -131,7 +135,8 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
unsigned int i, hsize, hmask, entries, prev_val, *hash_count;
const unsigned char *data, *buffer = buf;
struct delta_index *index;
- struct index_entry *entry, **hash;
+ struct unpacked_index_entry *entry, **hash;
+ struct index_entry *packed_entry, **packed_hash;
void *mem;
unsigned long memsize;
@@ -148,28 +153,21 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
hmask = hsize - 1;
/* allocate lookup index */
- memsize = sizeof(*index) +
- sizeof(*hash) * hsize +
+ memsize = sizeof(*hash) * hsize +
sizeof(*entry) * entries;
mem = malloc(memsize);
if (!mem)
return NULL;
- index = mem;
- mem = index + 1;
hash = mem;
mem = hash + hsize;
entry = mem;
- index->memsize = memsize;
- index->src_buf = buf;
- index->src_size = bufsize;
- index->hash_mask = hmask;
memset(hash, 0, hsize * sizeof(*hash));
/* allocate an array to count hash entries */
hash_count = calloc(hsize, sizeof(*hash_count));
if (!hash_count) {
- free(index);
+ free(hash);
return NULL;
}
@@ -183,12 +181,13 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
val = ((val << 8) | data[i]) ^ T[val >> RABIN_SHIFT];
if (val == prev_val) {
/* keep the lowest of consecutive identical blocks */
- entry[-1].ptr = data + RABIN_WINDOW;
+ entry[-1].entry.ptr = data + RABIN_WINDOW;
+ --entries;
} else {
prev_val = val;
i = val & hmask;
- entry->ptr = data + RABIN_WINDOW;
- entry->val = val;
+ entry->entry.ptr = data + RABIN_WINDOW;
+ entry->entry.val = val;
entry->next = hash[i];
hash[i] = entry++;
hash_count[i]++;
@@ -212,16 +211,59 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
continue;
entry = hash[i];
do {
- struct index_entry *keep = entry;
+ struct unpacked_index_entry *keep = entry;
int skip = hash_count[i] / HASH_LIMIT;
do {
+ --entries;
entry = entry->next;
} while(--skip && entry);
+ ++entries;
keep->next = entry;
} while(entry);
}
free(hash_count);
+ /* Now create the packed index in array form rather than
+ * linked lists */
+
+ memsize = sizeof(*index)
+ + sizeof(*packed_hash) * (hsize+1)
+ + sizeof(*packed_entry) * entries;
+
+ mem = malloc(memsize);
+
+ if (!mem) {
+ free(hash);
+ return NULL;
+ }
+
+ index = mem;
+ index->memsize = memsize;
+ index->src_buf = buf;
+ index->src_size = bufsize;
+ index->hash_mask = hmask;
+
+ mem = index + 1;
+ packed_hash = mem;
+ mem = packed_hash + (hsize+1);
+ packed_entry = mem;
+
+ /* Coalesce all entries belonging to one linked list into
+ * consecutive array entries */
+
+ for (i = 0; i < hsize; i++) {
+ packed_hash[i] = packed_entry;
+ for (entry = hash[i]; entry; entry = entry->next)
+ *packed_entry++ = entry->entry;
+ }
+
+ /* Sentinel value to indicate the length of the last hash
+ * bucket */
+
+ packed_hash[hsize] = packed_entry;
+ assert(packed_entry - (struct index_entry *)mem == entries);
+ free(hash);
+
return index;
}
@@ -302,7 +344,7 @@ create_delta(const struct delta_index *index,
val ^= U[data[-RABIN_WINDOW]];
val = ((val << 8) | *data) ^ T[val >> RABIN_SHIFT];
i = val & index->hash_mask;
- for (entry = index->hash[i]; entry; entry = entry->next) {
+ for (entry = index->hash[i]; entry < index->hash[i+1]; entry++) {
const unsigned char *ref = entry->ptr;
const unsigned char *src = data;
unsigned int ref_size = ref_top - ref;
--
1.5.3.GIT
^ permalink raw reply related
* [PATCH] diff-delta.c: Rationalize culling of hash buckets
From: David Kastrup @ 2007-09-08 21:25 UTC (permalink / raw)
To: git
In-Reply-To: <851wd8ua9z.fsf@lola.goethe.zz>
The previous hash bucket culling resulted in a somewhat unpredictable
number of hash bucket entries in the order of magnitude of HASH_LIMIT.
Replace this with a Bresenham-like algorithm leaving us with exactly
HASH_LIMIT entries by uniform culling.
Signed-off-by: David Kastrup <dak@gnu.org>
---
This is to be applied on top of the patch for packing the index
structure. Which is the reason it is posted as a followup to the
respective article. In contrast to that patch, this patch has only
been tested/reviewed by myself as to yet.
diff-delta.c | 41 +++++++++++++++++++++++++++++++----------
1 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/diff-delta.c b/diff-delta.c
index d355e5e..1a8d6fb 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -207,19 +207,40 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
* the reference buffer.
*/
for (i = 0; i < hsize; i++) {
- if (hash_count[i] < HASH_LIMIT)
+ int acc;
+
+ if (hash_count[i] <= HASH_LIMIT)
continue;
+
+ entries -= hash_count[i] - HASH_LIMIT;
+ /* We leave exactly HASH_LIMIT entries in the bucket */
+
entry = hash[i];
+ acc = 0;
do {
- struct unpacked_index_entry *keep = entry;
- int skip = hash_count[i] / HASH_LIMIT;
- do {
- --entries;
- entry = entry->next;
- } while(--skip && entry);
- ++entries;
- keep->next = entry;
- } while(entry);
+ acc += hash_count[i] - HASH_LIMIT;
+ if (acc > 0) {
+ struct unpacked_index_entry *keep = entry;
+ do {
+ entry = entry->next;
+ acc -= HASH_LIMIT;
+ } while (acc > 0);
+ keep->next = entry->next;
+ }
+ entry = entry->next;
+ } while (entry);
+
+ /* Assume that this loop is gone through exactly
+ * HASH_LIMIT times and is entered and left with
+ * acc==0. So the first statement in the loop
+ * contributes (hash_count[i]-HASH_LIMIT)*HASH_LIMIT
+ * to the accumulator, and the inner loop consequently
+ * is run (hash_count[i]-HASH_LIMIT) times, removing
+ * one element from the list each time. Since acc
+ * balances out to 0 at the final run, the inner loop
+ * body can't be left with entry==NULL. So we indeed
+ * encounter entry==NULL in the outer loop only.
+ */
}
free(hash_count);
--
1.5.3.GIT
^ permalink raw reply related
* Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.
From: Alex Riesen @ 2007-09-08 22:20 UTC (permalink / raw)
To: Junio C Hamano
Cc: Pierre Habouzit, Johannes Sixt, Keith Packard, Git Mailing List
In-Reply-To: <7vabry43cg.fsf@gitster.siamese.dyndns.org>
Junio C Hamano, Fri, Sep 07, 2007 22:39:43 +0200:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > I'd have added though that maybe update-ref should print a warning for
> > the references that do not match the restriction Junio added. This could
> > be done using the function Junio proposed un update_ref() in refs.c
>
> I would even suggest making it into an error, even if we do not
> error out on the reading side (being liberal when reading but
> more strict when creating, that is).
I agree (and suggest failing even on reading), but see below
> That confused_ref() needs to be tightened further, by the way.
> It is called only when we are considering to tack the user
> string immediately below $GIT_DIR/ so the only valid cases are
> (1) the string begins with "refs/",
If that will be the case git-p4-import.bat (yes, just a script of
mine) will break because it has its namespace directly in $GIT_DIR
(i.e. .git/p4/*) and stores there backup references. It is just a
someones (ok, it is mine) script, but maybe there are others, who
expect that plumbing level git-update-ref just do what its told.
> or (2) the string is all uppercase (or underscore), especially
> without slash.
I'd suggest just check for uppercase+underscore _or_ slash. It is
plumbing after all.
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Andreas Ericsson @ 2007-09-09 1:36 UTC (permalink / raw)
To: Pierre Habouzit, Andreas Ericsson, Walter Bright, git
In-Reply-To: <20070909003718.GE13385@artemis.corp>
Pierre Habouzit wrote:
> On Sat, Sep 08, 2007 at 11:50:34PM +0000, Andreas Ericsson wrote:
>
>>>> You can tell C compilers to
>>>> check all array accesses, but that is a performance issue.
>>> Runtime checking of arrays in D is a performance issue too, so it is
>>> selectable via a command line switch.
>> Same as in C then.
>
> HAHAHAHAHAHA. Please, who do you try to convince here ? Except in the
> local scope, there is few differences between a foo* and a foo[] in C.
>
"Runtime checking of arrays is a performance issue." It's true whether it's
done manually by the coder or by the compiler. The difference is that in C,
you get to choose where it should be done.
>>> But more importantly,
>>> 2) For dynamically sized arrays, the dimension of the array is carried
>>> with the array, so loops automatically loop the correct number of times.
>>> No runtime check is necessary, and it's easier for the code reviewer to
>>> visually check the code for correctness.
>> But this introduces handy but, strictly speaking, unnecessary overhead
>> as well, meaning, in short; 'D is slower than C, but easier to write
>> code in'.
>
> That's BS. See the strbuf API I've been pushing recently ? It has
> simplified git's code a lot, because each time git had to deal with a
> growing string, it had to deal with at least three variables: the buffer
> pointer, the current occupied length, and its allocated size. That was
> three thing to have variable names for, and to pass to functions.
>
Yup. I applaud your efforts, but it does come with a slight overhead,
except where it replaces faulty code. In practice, it's probably better
to use the api for all the string-handling, as none of it is performance-
critical.
> Now instead, it's just one struct. D gives that gratis. There is no
> performance loss because you _need_ to do the same. How do you deal with
> dynamic arrays if you dont't store their lenght and size somewhere ? Or
> are you the kind of programmer that write:
>
> /* 640kb should be enough for everyone… */
> some_type *array = malloc(640 << 10);
>
No, but it would depend on what I am to do with it.
>
>> So in essence, it's a bit like Python, but a teensy bit faster and a
>> lot easier to shoot yourself in the foot with.
>
>> What was the niche you were going for when you thought up D? It can't
>> have been systems programming, because *any* extra baggage is baggage
>> one would like to get rid of. If it was application programming I fail
>> to see how one more language would help, as there will be portability
>> problems galore and it's still considerably slower to develop in than
>> fe Python, while at the same time being considerably easier to mess up
>> in.
>
> Right now I'm just laughing. There is for sure overheads in some
> places of D, but the example you take, and what you try to attack in D
> is definitely not where you lose any kind of performance. You could have
> attacked the GC instead (which is after all an easy classical target).
>
I was asking what role D was designed to fill. I didn't mean it as an
attack, but re-reading what I wrote earlier I see it came off a bit harsh.
> Just to evaluate the silliness of your arguments:
> * http://www.digitalmars.com/d/comparison.html so that you can tell
> what the D features really are,
You may notice that the feature-list is being provided by the creators
and marketeers of the D language. Walter Bright certainly seems like a
nice enough person, but it's possible it's a tad biased.
> * http://shootout.alioth.debian.org/gp4/benchmark.php?test=all&lang=all
> so that you can know what the D performance really is about. Of
> course those are only micro benchmarks, but well, python is "just"
> 15 times slower than D, and D seems to be 10% slower.
I get it to 7.7xC and 1.2xC, respectively, but whatever. It still means
performance-critical apps will be written in C, while
insert-script-language-of-choice will still be used for prototyping and
not-so performance-critical apps.
> Well then I'm
> okay with D, I'm ready to buy 10% faster CPUs and avoid a lot of
> painful debugging time. In my world, 10% faster hardware is cheaper
> by many orders of magnitude than skilled programmers, but YMMV.
>
I'm curious as to how many fewer bugs D developers write compared to C
programmers. I guess it's hard to do a fair test given the comparatively
shallow pool of D gurus around, but it'd still be interesting to see a
practical test. 20% increase in runtime is certainly acceptable for
never having to see a bug again, but is it acceptable for 10% fewer bugs?
Or 20% fewer?
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* [PATCH] git-tag -s must fail if gpg cannot sign the tag.
From: Carlos Rica @ 2007-09-09 1:37 UTC (permalink / raw)
To: git, Junio C Hamano, Shawn O. Pearce, Johannes Schindelin
If the user has misconfigured `user.signingkey` in their .git/config
or just doesn't have any secret keys on their keyring and they ask
for a signed tag with `git tag -s` we better make sure the resulting
tag was actually signed by gpg.
Prior versions of builtin git-tag allowed this failure to slip
by without error as they were not checking the return value of
the finish_command() so they did not notice when gpg exited with
an error exit status. They also did not fail if gpg produced an
empty output or if read_in_full received an error from the read
system call while trying to read the pipe back from gpg.
Finally, we did not actually honor any return value from the do_sign
function as it returns ssize_t but was being stored into an unsigned
long. This caused the compiler to optimize out the die condition,
allowing git-tag to continue along and create the tag object.
However, when gpg gets a wrong username, it exits before any read was done
and then the writing process receives SIGNPIPE and program is terminated.
By ignoring this signal, anyway, the function write_or_die gets EPIPE from
write_in_full and exits returning 0 to the system without a message.
Here we better call to write_in_full directly so the next read can fail
printing a message and return safely to the caller.
With these issues fixed `git-tag -s` will now fail to create the
tag and will report a non-zero exit status to its caller, thereby
allowing automated helper scripts to detect (and recover from)
failure if gpg is not working properly.
Signed-off-by: Carlos Rica <jasampler@gmail.com>
---
This patch was initially sent by Shawn in a previous email:
http://marc.info/?l=git&m=118905254711808&w=2
I made many tests trying to know the reason for the problem
and these changes are the result. I'm not sure how to send this
right, because the patch was almost entirely from Shawn and I
only changed a few things. Please, see if the new test now pass
for everybody. Comments about the change are also welcomed.
I always CC to my mentor, Johannes Sch., because that was
usual when the GSoC project was active, so I continue
doing it that way to make him able to track my contributions.
Tell me if that's no longer necessary or it'is anoying.
builtin-tag.c | 14 ++++++++++----
t/t7004-tag.sh | 7 +++++++
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/builtin-tag.c b/builtin-tag.c
index 348919c..c9be69a 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -200,6 +200,10 @@ static ssize_t do_sign(char *buffer, size_t size, size_t max)
bracket[1] = '\0';
}
+ /* When the username signingkey is bad, program could be terminated
+ * because gpg exits without reading and then write gets SIGNPIPE. */
+ signal(SIGPIPE, SIG_IGN);
+
memset(&gpg, 0, sizeof(gpg));
gpg.argv = args;
gpg.in = -1;
@@ -212,12 +216,13 @@ static ssize_t do_sign(char *buffer, size_t size, size_t max)
if (start_command(&gpg))
return error("could not run gpg.");
- write_or_die(gpg.in, buffer, size);
+ write_in_full(gpg.in, buffer, size);
close(gpg.in);
gpg.close_in = 0;
len = read_in_full(gpg.out, buffer + size, max - size);
- finish_command(&gpg);
+ if (finish_command(&gpg) || !len || len < 0)
+ return error("gpg failed to sign the tag");
if (len == max - size)
return error("could not read the entire signature from gpg.");
@@ -310,9 +315,10 @@ static void create_tag(const unsigned char *object, const char *tag,
size += header_len;
if (sign) {
- size = do_sign(buffer, size, max_size);
- if (size < 0)
+ ssize_t r = do_sign(buffer, size, max_size);
+ if (r < 0)
die("unable to sign the tag");
+ size = r;
}
if (write_sha1_file(buffer, size, tag_type, result) < 0)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 606d4f2..0d07bc3 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -990,6 +990,13 @@ test_expect_success \
git diff expect actual
'
+# try to sign with bad user.signingkey
+git config user.signingkey BobTheMouse
+test_expect_failure \
+ 'git-tag -s fails if gpg is misconfigured' \
+ 'git tag -s -m tail tag-gpg-failure'
+git config --unset user.signingkey
+
# try to verify without gpg:
rm -rf gpghome
--
1.5.0
^ permalink raw reply related
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Pierre Habouzit @ 2007-09-09 0:37 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Walter Bright, git
In-Reply-To: <46E3354A.7030407@op5.se>
[-- Attachment #1: Type: text/plain, Size: 4510 bytes --]
On Sat, Sep 08, 2007 at 11:50:34PM +0000, Andreas Ericsson wrote:
> Walter Bright wrote:
> > David Kastrup wrote:
> > > Again, C won't keep you from shooting yourself in the foot.
> > Right, it won't. A good systems language should do what it can to
> > prevent the programmer from *inadvertently* shooting himself in the
> > foot, while allowing him to *deliberately* shoot himself in the foot.
>
> No, a good systems language should do exactly what it's told.
> Supporting tools should tell the programmer if he's risking shooting
> himself in the foot.
I beg to differ. I mean, knowing enough of D, I think that what Walter
tries to say is that a good language should provide constructions that
when used prevent the programmer to shoot himself in both foot at the
same time.
D supports most of the C constructions, so when you want to juggle
with razor blades, you're free to do so in D. Though, the language
provides idioms that prevent you to write stupid mistakes when used. And
that is great.
D is not Java, you have pointers, you can deal with memory
explicitely, you can do whatever you can do in C with no or very little
overhead. Or you can use higher level D, at your own discretion.
> > > You can tell C compilers to
> > > check all array accesses, but that is a performance issue.
> > Runtime checking of arrays in D is a performance issue too, so it is
> > selectable via a command line switch.
>
> Same as in C then.
HAHAHAHAHAHA. Please, who do you try to convince here ? Except in the
local scope, there is few differences between a foo* and a foo[] in C.
> > But more importantly,
> > 2) For dynamically sized arrays, the dimension of the array is carried
> > with the array, so loops automatically loop the correct number of times.
> > No runtime check is necessary, and it's easier for the code reviewer to
> > visually check the code for correctness.
>
> But this introduces handy but, strictly speaking, unnecessary overhead
> as well, meaning, in short; 'D is slower than C, but easier to write
> code in'.
That's BS. See the strbuf API I've been pushing recently ? It has
simplified git's code a lot, because each time git had to deal with a
growing string, it had to deal with at least three variables: the buffer
pointer, the current occupied length, and its allocated size. That was
three thing to have variable names for, and to pass to functions.
Now instead, it's just one struct. D gives that gratis. There is no
performance loss because you _need_ to do the same. How do you deal with
dynamic arrays if you dont't store their lenght and size somewhere ? Or
are you the kind of programmer that write:
/* 640kb should be enough for everyone… */
some_type *array = malloc(640 << 10);
> So in essence, it's a bit like Python, but a teensy bit faster and a
> lot easier to shoot yourself in the foot with.
> What was the niche you were going for when you thought up D? It can't
> have been systems programming, because *any* extra baggage is baggage
> one would like to get rid of. If it was application programming I fail
> to see how one more language would help, as there will be portability
> problems galore and it's still considerably slower to develop in than
> fe Python, while at the same time being considerably easier to mess up
> in.
Right now I'm just laughing. There is for sure overheads in some
places of D, but the example you take, and what you try to attack in D
is definitely not where you lose any kind of performance. You could have
attacked the GC instead (which is after all an easy classical target).
Just to evaluate the silliness of your arguments:
* http://www.digitalmars.com/d/comparison.html so that you can tell
what the D features really are,
* http://shootout.alioth.debian.org/gp4/benchmark.php?test=all&lang=all
so that you can know what the D performance really is about. Of
course those are only micro benchmarks, but well, python is "just"
15 times slower than D, and D seems to be 10% slower. Well then I'm
okay with D, I'm ready to buy 10% faster CPUs and avoid a lot of
painful debugging time. In my world, 10% faster hardware is cheaper
by many orders of magnitude than skilled programmers, but YMMV.
--
·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: [PATCH] diff-delta.c: rename {a,}{entry,hash} to {,u}{entry,hash}
From: Nicolas Pitre @ 2007-09-09 0:38 UTC (permalink / raw)
To: David Kastrup; +Cc: git
In-Reply-To: <85abrwuas8.fsf@lola.goethe.zz>
On Sat, 8 Sep 2007, David Kastrup wrote:
> Nicolas Pitre <nico@cam.org> writes:
>
> > On Sat, 8 Sep 2007, David Kastrup wrote:
> >
> >> The variables for the packed entries are now called just entry and
> >> hash rather than aentry+ahash, and those for the unpacked entries have
> >> been renamed to uentry and uhash from the original entry and hash.
> >>
> >> While this makes the diff to the unchanged code larger, it matches the
> >> type declarations better.
> >
> > Since the packed version is used less often, could you rename that one
> > instead, and use packed_entry and packed_hash forclarity.
> >
> > Also please fold this with your other patch (not the culling one).
>
> Uh, what other patch? If I don't do the renames in the second patch,
> then only the first patch remains. Which I'll change to use the
> proposed names. Hope that's what you meant.
Yes, that's what I meant.
> > Then you can add my ACK.
>
> I'll do that, and rework the culling patch on top of that (unacked for
> now).
I'm looking at it atm.
Nicolas
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Andreas Ericsson @ 2007-09-09 0:29 UTC (permalink / raw)
To: Dmitry Kakurin; +Cc: Linus Torvalds, Matthieu Moy, Git
In-Reply-To: <a1bbc6950709071517n7e7e99ffl3dd351092e7f19d6@mail.gmail.com>
Dmitry Kakurin wrote:
> On 9/6/07, Andreas Ericsson <ae@op5.se> wrote:
>> They already have, but every now and then someone comes along and suggest
>> a complete rewrite in some other language. So far we've had Java (there's
>> always one...), Python and now C++.
>
> Since this "complete rewrite" was mentioned in multiple emails I'd
> like to rectify that:
> What I'm offering (for Git) is to use C++ as a "better C".
> Don't change any existing *working* code, but start introducing simple
> C++ constructs in the new code.
> Git is simple enough to not require any high-level abstractions. But
> some utility classes could make code much simpler.
>
There are far too many highly valuable contributors that have spoken
against C++ for me to believe that C++ and C will ever co-exist in the
official git repo. Good thing utility classes can be developed on top
of the existing C-code, but in a separate repo, and packed into a
library. That way, you get some hacking ground for your beloved C++
coderswhile the current git contributors can keep contributing in the
language they like best.
> And BTW, I don't even like C++ that much :-), I just like it much
> better than C. I've been saying that C++ is a legacy language for
> quite some time now. But we will use it for many years to come because
> the size of this legacy code is huge, so there will be plenty of C++
> developers available (to contribute to Git :-).
The C code base is a lot larger and C++ will drop dead pretty fast if it's
ever removed or left unmaintained. So much for dinosaurs...
> And C++ is the only way to move with existing C codebase.
Complete and utter BS. It can also stay in C, or get language bindings for
Python/Perl/PHP/LUA(?)/whatever, or both.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Andreas Ericsson @ 2007-09-09 0:25 UTC (permalink / raw)
To: Walter Bright; +Cc: git
In-Reply-To: <fbs8es$1cd$1@sea.gmane.org>
Walter Bright wrote:
> Andreas Ericsson wrote:
>> Walter Bright wrote:
>>> 1) You wind up having to implement the complex, dirty details of
>>> things yourself. The consequences of this are:
>>>
>>> a) you pick a simpler algorithm (which is likely less efficient -
>>> I run across bubble sorts all the time in code)
>>>
>>> b) once you implement, tune, and squeeze all the bugs out of those
>>> complex, dirty details, you're reluctant to change it. You're
>>> reluctant to try a different algorithm to see if it's faster. I've
>>> seen this effect a lot in my own code. (I translated a large body of
>>> my own C++ code that I'd spent months tuning to D, and quickly
>>> managed to get significantly more speed out of it, because it was
>>> much simpler to try out different algorithms/data structures.)
>>>
>>
>> I haven't seen this in the development of git, although to be fair, you
>> didn't mention the number of developers that were simultaneously working
>> on your project.
>
> On my project, one. But I've seen this problem repeatedly in other
> projects that had multiple developers. For example, I used to use
> version 1 of an assembler. It was itself written entirely in assembler.
> It ran *incredibly* slowly on large asm files. But it was written in
> assembler, which is very fast, so how could that be?
>
> Turns out, the symbol table used internally was a linear one. A linear
> symbol table is easy to implement, but doesn't scale well at all. A
> linear symbol table was implemented because it was just harder to do
> more advanced symbol table algorithms in assembler. In this case, a
> higher level language re-implementation made the assembler much faster,
> even though that implementation was SLOWER in every detail. It was
> faster overall, because it was easier to develop faster algorithms.
>
Well, when the ease-of-coding vs the exec-speed of D vs C is that of
C vs asm, C will be dead fairly soon. However, since C is so ingrained
in every language designer's head, I find that unlikely to happen any
time soon.
>
>> Opensource projects with many contributors (git, linux) work differently,
>> since one or a few among the plethora of authors will almost always be
>> a true expert at the problem being solved.
>
> That is a nice advantage. I don't think many projects can rely on having
> the best in the business working on them, though <g>.
>
True that. I know a fair few projects that could have done with borrowing
one or two proper gurus, but even opensource programmers are selfish in
that we usually only work for something that benefits ourselves.
>
>> The point is that, given enough developers, *someone* is bound to
>> find an algorithm that works so well that it's no longer worth
>> investing time to even discuss if anything else would work better,
>> either because it moves the performance bottleneck to somewhere else
>> (where further speedups would no longer produce humanly measurable
>> improvements), or because the action seems instantanous to the user
>> (further improvements simply aren't worth it, because no valuable
>> resource will be saved from it).
>
> Sure, but I suggest that few projects reach this maxima.
True again, but given what I said above holds, it would be madness to
move from the lingua franca of oss hacking to a less common one, as it
would mean fewer eyes on the code.
> Case in point:
> ld, the gnu linker. It's terribly slow. To see how slow it is, compare
> it to optlink (the 15 years old one that comes with D for Windows). So I
> don't believe there is anything inherent about linking that should make
> ld so slow. There's some huge leverage possible in speeding up ld
> (spreading out that saved time among all the gnu developers).
>
> So while git may have reached a maxima in performance, I don't think
> this principle is applicable in general, even for very widely used open
> source projects that would profit greatly from improved performance.
>
Interesting. I recently did a spot of work comparing various string-hashing
algorithms. Perhaps I should head over to the ld camp and see if I can help.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] git-diff: don't squelch the new SHA1 in submodule diffs
From: Johannes Schindelin @ 2007-09-09 0:18 UTC (permalink / raw)
To: skimo; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20070908103022.GA15229MdfPADPa@greensroom.kotnet.org>
Hi,
On Sat, 8 Sep 2007, Sven Verdoolaege wrote:
> The code to squelch empty diffs introduced by commit
> fb13227e089f22dc31a3b1624559153821056848 would inadvertently
> populate filespec "two" of a submodule change using the uninitialized
> (null) SHA1, thereby replacing the submodule SHA1 by 0{40} in the output.
>
> This change teaches diffcore_skip_stat_unmatch to handle
> submodule changes correctly.
Ah, I was wondering... This struck me as odd in msysGit, but I did not
know if it was a problem I introduced in 4msysgit or not. Unfortunately,
I did not have the time to investigate further.
Thanks,
Dscho
^ permalink raw reply
* Re: Use more strbufs series [on top of next]
From: Pierre Habouzit @ 2007-09-09 0:12 UTC (permalink / raw)
To: git
In-Reply-To: <11892962763548-git-send-email-madcoder@debian.org>
[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]
On Sun, Sep 09, 2007 at 12:04:30AM +0000, Pierre Habouzit wrote:
> Here is a series of patches on top of next, which use strbufs in even
> more places. Most notably, it uses it in the commit pretty printer (and
> commit message formatter), and it supersedes definitely read_fd
> (previously in sha1_file.c).
A side note that could help the patch readers: in some places I've
made hunks that basically rename variables called space or size into
len. The reason is that (in my coding standards) length is the length of
the object we are referring to, whereas space/size/... is the amount of
memory we have allocated for it. For a string, the latter is strictly
greater than the former because of the ending NUL of course.
The rename is because my brain is used to this way of thinking and
that it helps knowing if the off-by-some (off-by-one for a string) are
included in the length-related number you work with or not. So please
don't discard those :)
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH 6/6] Replace all read_fd use with strbuf_read, and get rid of it.
From: Pierre Habouzit @ 2007-09-09 0:04 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <11892962771756-git-send-email-madcoder@debian.org>
This brings builtin-stripspace, builtin-tag and mktag to use strbufs.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-stripspace.c | 21 ++++++--------
builtin-tag.c | 78 ++++++++++++++++++++------------------------------
cache.h | 1 -
mktag.c | 15 ++++-----
sha1_file.c | 60 ++++++--------------------------------
5 files changed, 56 insertions(+), 119 deletions(-)
diff --git a/builtin-stripspace.c b/builtin-stripspace.c
index 916355c..4815f8a 100644
--- a/builtin-stripspace.c
+++ b/builtin-stripspace.c
@@ -1,5 +1,6 @@
#include "builtin.h"
#include "cache.h"
+#include "strbuf.h"
/*
* Returns the length of a line, without trailing spaces.
@@ -74,26 +75,22 @@ size_t stripspace(char *buffer, size_t length, int skip_comments)
int cmd_stripspace(int argc, const char **argv, const char *prefix)
{
- char *buffer;
- unsigned long size;
+ struct strbuf buf;
int strip_comments = 0;
if (argc > 1 && (!strcmp(argv[1], "-s") ||
!strcmp(argv[1], "--strip-comments")))
strip_comments = 1;
- size = 1024;
- buffer = xmalloc(size);
- if (read_fd(0, &buffer, &size)) {
- free(buffer);
+ strbuf_init(&buf);
+ if (strbuf_read(&buf, 0) < 0)
die("could not read the input");
- }
- size = stripspace(buffer, size, strip_comments);
- write_or_die(1, buffer, size);
- if (size)
- putc('\n', stdout);
+ strbuf_setlen(&buf, stripspace(buf.buf, buf.len, strip_comments));
+ if (buf.len)
+ strbuf_addch(&buf, '\n');
- free(buffer);
+ write_or_die(1, buf.buf, buf.len);
+ strbuf_release(&buf);
return 0;
}
diff --git a/builtin-tag.c b/builtin-tag.c
index 348919c..1a93037 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -8,6 +8,7 @@
#include "cache.h"
#include "builtin.h"
+#include "strbuf.h"
#include "refs.h"
#include "tag.h"
#include "run-command.h"
@@ -17,7 +18,7 @@ static const char builtin_tag_usage[] =
static char signingkey[1000];
-static void launch_editor(const char *path, char **buffer, unsigned long *len)
+static void launch_editor(const char *path, struct strbuf *buffer)
{
const char *editor, *terminal;
struct child_process child;
@@ -55,10 +56,8 @@ static void launch_editor(const char *path, char **buffer, unsigned long *len)
fd = open(path, O_RDONLY);
if (fd < 0)
die("could not open '%s': %s", path, strerror(errno));
- if (read_fd(fd, buffer, len)) {
- free(*buffer);
- die("could not read message file '%s': %s",
- path, strerror(errno));
+ if (strbuf_read(buffer, fd) < 0) {
+ die("could not read message file '%s': %s", path, strerror(errno));
}
close(fd);
}
@@ -184,7 +183,7 @@ static int verify_tag(const char *name, const char *ref,
return 0;
}
-static ssize_t do_sign(char *buffer, size_t size, size_t max)
+static int do_sign(struct strbuf *buffer)
{
struct child_process gpg;
const char *args[4];
@@ -212,17 +211,17 @@ static ssize_t do_sign(char *buffer, size_t size, size_t max)
if (start_command(&gpg))
return error("could not run gpg.");
- write_or_die(gpg.in, buffer, size);
+ write_or_die(gpg.in, buffer->buf, buffer->len);
close(gpg.in);
gpg.close_in = 0;
- len = read_in_full(gpg.out, buffer + size, max - size);
+ len = strbuf_read(buffer, gpg.out);
finish_command(&gpg);
- if (len == max - size)
+ if (len < 0)
return error("could not read the entire signature from gpg.");
- return size + len;
+ return 0;
}
static const char tag_template[] =
@@ -245,15 +244,13 @@ static int git_tag_config(const char *var, const char *value)
return git_default_config(var, value);
}
-#define MAX_SIGNATURE_LENGTH 1024
-/* message must be NULL or allocated, it will be reallocated and freed */
static void create_tag(const unsigned char *object, const char *tag,
- char *message, int sign, unsigned char *result)
+ struct strbuf *buf, int message, int sign,
+ unsigned char *result)
{
enum object_type type;
- char header_buf[1024], *buffer = NULL;
- int header_len, max_size;
- unsigned long size = 0;
+ char header_buf[1024];
+ int header_len;
type = sha1_object_info(object, NULL);
if (type <= OBJ_NONE)
@@ -285,52 +282,40 @@ static void create_tag(const unsigned char *object, const char *tag,
write_or_die(fd, tag_template, strlen(tag_template));
close(fd);
- launch_editor(path, &buffer, &size);
+ launch_editor(path, buf);
unlink(path);
free(path);
}
- else {
- buffer = message;
- size = strlen(message);
- }
- size = stripspace(buffer, size, 1);
+ strbuf_setlen(buf, stripspace(buf->buf, buf->len, 1));
- if (!message && !size)
+ if (!message && !buf->len)
die("no tag message?");
/* insert the header and add the '\n' if needed: */
- max_size = header_len + size + (sign ? MAX_SIGNATURE_LENGTH : 0) + 1;
- buffer = xrealloc(buffer, max_size);
- if (size)
- buffer[size++] = '\n';
- memmove(buffer + header_len, buffer, size);
- memcpy(buffer, header_buf, header_len);
- size += header_len;
-
- if (sign) {
- size = do_sign(buffer, size, max_size);
- if (size < 0)
- die("unable to sign the tag");
- }
+ if (buf->len)
+ strbuf_addch(buf, '\n');
+ strbuf_insert(buf, 0, header_buf, header_len);
- if (write_sha1_file(buffer, size, tag_type, result) < 0)
+ if (sign && do_sign(buf) < 0)
+ die("unable to sign the tag");
+ if (write_sha1_file(buf->buf, buf->len, tag_type, result) < 0)
die("unable to write tag file");
- free(buffer);
}
int cmd_tag(int argc, const char **argv, const char *prefix)
{
+ struct strbuf buf;
unsigned char object[20], prev[20];
- int annotate = 0, sign = 0, force = 0, lines = 0;
- char *message = NULL;
+ int annotate = 0, sign = 0, force = 0, lines = 0, message = 0;
char ref[PATH_MAX];
const char *object_ref, *tag;
int i;
struct ref_lock *lock;
git_config(git_tag_config);
+ strbuf_init(&buf);
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
@@ -366,11 +351,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die("option -m needs an argument.");
if (message)
die("only one -F or -m option is allowed.");
- message = xstrdup(argv[i]);
+ strbuf_addstr(&buf, argv[i]);
+ message = 1;
continue;
}
if (!strcmp(arg, "-F")) {
- unsigned long len;
int fd;
annotate = 1;
@@ -388,12 +373,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die("could not open '%s': %s",
argv[i], strerror(errno));
}
- len = 1024;
- message = xmalloc(len);
- if (read_fd(fd, &message, &len)) {
- free(message);
+ if (strbuf_read(&buf, fd) < 0) {
die("cannot read %s", argv[i]);
}
+ message = 1;
continue;
}
if (!strcmp(arg, "-u")) {
@@ -441,7 +424,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die("tag '%s' already exists", tag);
if (annotate)
- create_tag(object, tag, message, sign, object);
+ create_tag(object, tag, &buf, message, sign, object);
lock = lock_any_ref_for_update(ref, prev, 0);
if (!lock)
@@ -449,5 +432,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (write_ref_sha1(lock, object, NULL) < 0)
die("%s: cannot update the ref", ref);
+ strbuf_release(&buf);
return 0;
}
diff --git a/cache.h b/cache.h
index 493983c..231f81d 100644
--- a/cache.h
+++ b/cache.h
@@ -269,7 +269,6 @@ extern int ie_match_stat(struct index_state *, struct cache_entry *, struct stat
extern int ie_modified(struct index_state *, struct cache_entry *, struct stat *, int);
extern int ce_path_match(const struct cache_entry *ce, const char **pathspec);
extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object, enum object_type type, const char *path);
-extern int read_fd(int fd, char **return_buf, unsigned long *return_size);
extern int index_pipe(unsigned char *sha1, int fd, const char *type, int write_object);
extern int index_path(unsigned char *sha1, const char *path, struct stat *st, int write_object);
extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
diff --git a/mktag.c b/mktag.c
index 38acd5a..0c29a2b 100644
--- a/mktag.c
+++ b/mktag.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "strbuf.h"
#include "tag.h"
/*
@@ -111,8 +112,7 @@ static int verify_tag(char *buffer, unsigned long size)
int main(int argc, char **argv)
{
- unsigned long size = 4096;
- char *buffer = xmalloc(size);
+ struct strbuf buf;
unsigned char result_sha1[20];
if (argc != 1)
@@ -120,21 +120,20 @@ int main(int argc, char **argv)
setup_git_directory();
- if (read_fd(0, &buffer, &size)) {
- free(buffer);
+ strbuf_init(&buf);
+ if (strbuf_read(&buf, 0) < 0) {
die("could not read from stdin");
}
/* Verify it for some basic sanity: it needs to start with
"object <sha1>\ntype\ntagger " */
- if (verify_tag(buffer, size) < 0)
+ if (verify_tag(buf.buf, buf.len) < 0)
die("invalid tag signature file");
- if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0)
+ if (write_sha1_file(buf.buf, buf.len, tag_type, result_sha1) < 0)
die("unable to write tag file");
- free(buffer);
-
+ strbuf_release(&buf);
printf("%s\n", sha1_to_hex(result_sha1));
return 0;
}
diff --git a/sha1_file.c b/sha1_file.c
index 9978a58..d011056 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -14,6 +14,7 @@
#include "tag.h"
#include "tree.h"
#include "refs.h"
+#include "strbuf.h"
#ifndef O_NOATIME
#if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
@@ -2302,68 +2303,25 @@ int has_sha1_file(const unsigned char *sha1)
return find_sha1_file(sha1, &st) ? 1 : 0;
}
-/*
- * reads from fd as long as possible into a supplied buffer of size bytes.
- * If necessary the buffer's size is increased using realloc()
- *
- * returns 0 if anything went fine and -1 otherwise
- *
- * The buffer is always NUL-terminated, not including it in returned size.
- *
- * NOTE: both buf and size may change, but even when -1 is returned
- * you still have to free() it yourself.
- */
-int read_fd(int fd, char **return_buf, unsigned long *return_size)
-{
- char *buf = *return_buf;
- unsigned long size = *return_size;
- ssize_t iret;
- unsigned long off = 0;
-
- if (!buf || size <= 1) {
- size = 1024;
- buf = xrealloc(buf, size);
- }
-
- do {
- iret = xread(fd, buf + off, (size - 1) - off);
- if (iret > 0) {
- off += iret;
- if (off == size - 1) {
- size = alloc_nr(size);
- buf = xrealloc(buf, size);
- }
- }
- } while (iret > 0);
-
- buf[off] = '\0';
-
- *return_buf = buf;
- *return_size = off;
-
- if (iret < 0)
- return -1;
- return 0;
-}
-
int index_pipe(unsigned char *sha1, int fd, const char *type, int write_object)
{
- unsigned long size = 4096;
- char *buf = xmalloc(size);
+ struct strbuf buf;
int ret;
- if (read_fd(fd, &buf, &size)) {
- free(buf);
+ strbuf_init(&buf);
+ if (strbuf_read(&buf, fd) < 0) {
+ strbuf_release(&buf);
return -1;
}
if (!type)
type = blob_type;
if (write_object)
- ret = write_sha1_file(buf, size, type, sha1);
+ ret = write_sha1_file(buf.buf, buf.len, type, sha1);
else
- ret = hash_sha1_file(buf, size, type, sha1);
- free(buf);
+ ret = hash_sha1_file(buf.buf, buf.len, type, sha1);
+ strbuf_release(&buf);
+
return ret;
}
--
1.5.3.1
^ permalink raw reply related
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Andreas Ericsson @ 2007-09-09 0:09 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: Johannes Schindelin, Dmitry Kakurin, Matthieu Moy, Git,
Linus Torvalds
In-Reply-To: <03C31350-67A5-4B94-A841-5CEB37E78DE8@wincent.com>
Wincent Colaiuta wrote:
> El 7/9/2007, a las 15:58, Andreas Ericsson escribió:
>
>> Yes, but that's what I said in the original email as well. C is just so
>> much more pleasant to write in that the only place you'd (sanely) use
>> asm is in exactly these tight loops, where the code is likely to be used
>> and reused until the algorithm it describes is no longer a viable option
>> for doing what it was originally designed to do.
>>
>> It still proves the point though, as surely as n+1 > n for any value
>> of n:
>> Hand-optimized assembly is faster than compiler-optimized C code.
>
> In a theoretical ideal world, yes; no one would argue that C is faster
> than fine-tuned assembly.
>
> But in the *real world* rewriting Git in assembly would be like painting
> a house using a single horse hair instead of a paint brush or roller.
> Your SHA-1 example is a perfect example of where you benefit from doing
> a tiny embellished detail using the single hair (assembly) and leave all
> the rest in C.
>
> In the real world and not the theoretical ideal world, it's not just
> about the diminishing returns you get from writing more and more of a
> code base in assembly instead of just the performance-critical
> bottlenecks; it's that you're more likely to make subtle mistakes or
> even make things slower. GCC does a remarkable job of optimizing in a
> huge number of use cases, and best of all, it does it for free. Personal
> opinion, of course, but that's the way I think it is.
>
The discussion was theoretical from the beginning. Nobody's arguing that
git should be rewritten in asm, and you've been preaching to the choir far
too long now. I'll just drop this thread.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* [PATCH 3/6] Rework pretty_print_commit to use strbufs instead of custom buffers.
From: Pierre Habouzit @ 2007-09-09 0:04 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <11892962764194-git-send-email-madcoder@debian.org>
Also remove the "len" parameter, as:
(1) it was used as a max boundary, and every caller used ~0u
(2) we check for final NUL no matter what, so it doesn't help for speed.
As a result most of the pp_* function takes 3 arguments less, and we need
a lot less local variables, this makes the code way more readable, and
easier to extend if needed.
This patch also fixes some spacing and cosmetic issues.
This patch also fixes (as a side effect) a memory leak intoruced in
builtin-archive.c at commit df4a394f (fmt was xmalloc'ed and not free'd)
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-archive.c | 38 +++----
builtin-branch.c | 15 +--
builtin-log.c | 12 +-
builtin-rev-list.c | 13 +-
builtin-show-branch.c | 13 +-
commit.c | 330 ++++++++++++++++++-------------------------------
commit.h | 9 +-
log-tree.c | 56 +++------
8 files changed, 189 insertions(+), 297 deletions(-)
diff --git a/builtin-archive.c b/builtin-archive.c
index e221f11..0a41ad7 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -84,14 +84,16 @@ static int run_remote_archiver(const char *remote, int argc,
static void *format_subst(const struct commit *commit, const char *format,
unsigned long *sizep)
{
- unsigned long len = *sizep, result_len = 0;
+ unsigned long len = *sizep;
const char *a = format;
- char *result = NULL;
+ struct strbuf result;
+ struct strbuf fmt;
+
+ strbuf_init(&result);
+ strbuf_init(&fmt);
for (;;) {
const char *b, *c;
- char *fmt, *formatted = NULL;
- unsigned long a_len, fmt_len, formatted_len, allocated = 0;
b = memmem(a, len, "$Format:", 8);
if (!b || a + len < b + 9)
@@ -100,31 +102,23 @@ static void *format_subst(const struct commit *commit, const char *format,
if (!c)
break;
- a_len = b - a;
- fmt_len = c - b - 8;
- fmt = xmalloc(fmt_len + 1);
- memcpy(fmt, b + 8, fmt_len);
- fmt[fmt_len] = '\0';
-
- formatted_len = format_commit_message(commit, fmt, &formatted,
- &allocated);
- result = xrealloc(result, result_len + a_len + formatted_len);
- memcpy(result + result_len, a, a_len);
- memcpy(result + result_len + a_len, formatted, formatted_len);
- result_len += a_len + formatted_len;
+ strbuf_reset(&fmt);
+ strbuf_add(&fmt, b + 8, c - b - 8);
+
+ strbuf_add(&result, a, b - a);
+ format_commit_message(commit, fmt.buf, &result);
len -= c + 1 - a;
a = c + 1;
}
- if (result && len) {
- result = xrealloc(result, result_len + len);
- memcpy(result + result_len, a, len);
- result_len += len;
+ if (result.len && len) {
+ strbuf_add(&result, a, len);
}
- *sizep = result_len;
+ *sizep = result.len;
- return result;
+ strbuf_release(&fmt);
+ return strbuf_detach(&result);
}
static void *convert_to_archive(const char *path,
diff --git a/builtin-branch.c b/builtin-branch.c
index 5f5c182..68cd5f9 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -268,23 +268,22 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
}
if (verbose) {
- char *subject = NULL;
- unsigned long subject_len = 0;
+ struct strbuf subject;
const char *sub = " **** invalid ref ****";
+ strbuf_init(&subject);
+
commit = lookup_commit(item->sha1);
if (commit && !parse_commit(commit)) {
- pretty_print_commit(CMIT_FMT_ONELINE, commit, ~0,
- &subject, &subject_len, 0,
- NULL, NULL, 0);
- sub = subject;
+ pretty_print_commit(CMIT_FMT_ONELINE, commit,
+ &subject, 0, NULL, NULL, 0);
+ sub = subject.buf;
}
printf("%c %s%-*s%s %s %s\n", c, branch_get_color(color),
maxwidth, item->name,
branch_get_color(COLOR_BRANCH_RESET),
find_unique_abbrev(item->sha1, abbrev), sub);
- if (subject)
- free(subject);
+ strbuf_release(&subject);
} else {
printf("%c %s%s%s\n", c, branch_get_color(color), item->name,
branch_get_color(COLOR_BRANCH_RESET));
diff --git a/builtin-log.c b/builtin-log.c
index fa81c25..21b35a1 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -763,13 +763,13 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
sign = '-';
if (verbose) {
- char *buf = NULL;
- unsigned long buflen = 0;
- pretty_print_commit(CMIT_FMT_ONELINE, commit, ~0,
- &buf, &buflen, 0, NULL, NULL, 0);
+ struct strbuf buf;
+ strbuf_init(&buf);
+ pretty_print_commit(CMIT_FMT_ONELINE, commit,
+ &buf, 0, NULL, NULL, 0);
printf("%c %s %s\n", sign,
- sha1_to_hex(commit->object.sha1), buf);
- free(buf);
+ sha1_to_hex(commit->object.sha1), buf.buf);
+ strbuf_release(&buf);
}
else {
printf("%c %s\n", sign,
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index ac551d5..7967f86 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -80,13 +80,12 @@ static void show_commit(struct commit *commit)
putchar('\n');
if (revs.verbose_header) {
- char *buf = NULL;
- unsigned long buflen = 0;
- pretty_print_commit(revs.commit_format, commit, ~0,
- &buf, &buflen,
- revs.abbrev, NULL, NULL, revs.date_mode);
- printf("%s%c", buf, hdr_termination);
- free(buf);
+ struct strbuf buf;
+ strbuf_init(&buf);
+ pretty_print_commit(revs.commit_format, commit,
+ &buf, revs.abbrev, NULL, NULL, revs.date_mode);
+ printf("%s%c", buf.buf, hdr_termination);
+ strbuf_release(&buf);
}
maybe_flush_or_die(stdout, "stdout");
if (commit->parents) {
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 4fa87f6..020116f 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -259,16 +259,15 @@ static void join_revs(struct commit_list **list_p,
static void show_one_commit(struct commit *commit, int no_name)
{
- char *pretty = NULL;
+ struct strbuf pretty;
const char *pretty_str = "(unavailable)";
- unsigned long pretty_len = 0;
struct commit_name *name = commit->util;
+ strbuf_init(&pretty);
if (commit->object.parsed) {
- pretty_print_commit(CMIT_FMT_ONELINE, commit, ~0,
- &pretty, &pretty_len,
- 0, NULL, NULL, 0);
- pretty_str = pretty;
+ pretty_print_commit(CMIT_FMT_ONELINE, commit,
+ &pretty, 0, NULL, NULL, 0);
+ pretty_str = pretty.buf;
}
if (!prefixcmp(pretty_str, "[PATCH] "))
pretty_str += 8;
@@ -289,7 +288,7 @@ static void show_one_commit(struct commit *commit, int no_name)
find_unique_abbrev(commit->object.sha1, 7));
}
puts(pretty_str);
- free(pretty);
+ strbuf_release(&pretty);
}
static char *ref_name[MAX_REVS + 1];
diff --git a/commit.c b/commit.c
index 25781cc..02dfd2d 100644
--- a/commit.c
+++ b/commit.c
@@ -458,11 +458,11 @@ void clear_commit_marks(struct commit *commit, unsigned int mark)
/*
* Generic support for pretty-printing the header
*/
-static int get_one_line(const char *msg, unsigned long len)
+static int get_one_line(const char *msg)
{
int ret = 0;
- while (len--) {
+ for (;;) {
char c = *msg++;
if (!c)
break;
@@ -485,31 +485,24 @@ static int is_rfc2047_special(char ch)
return (non_ascii(ch) || (ch == '=') || (ch == '?') || (ch == '_'));
}
-static int add_rfc2047(char *buf, const char *line, int len,
+static void add_rfc2047(struct strbuf *sb, const char *line, int len,
const char *encoding)
{
- char *bp = buf;
- int i, needquote;
- char q_encoding[128];
- const char *q_encoding_fmt = "=?%s?q?";
+ int i, last;
- for (i = needquote = 0; !needquote && i < len; i++) {
+ for (i = 0; i < len; i++) {
int ch = line[i];
if (non_ascii(ch))
- needquote++;
- if ((i + 1 < len) &&
- (ch == '=' && line[i+1] == '?'))
- needquote++;
+ goto needquote;
+ if ((i + 1 < len) && (ch == '=' && line[i+1] == '?'))
+ goto needquote;
}
- if (!needquote)
- return sprintf(buf, "%.*s", len, line);
-
- i = snprintf(q_encoding, sizeof(q_encoding), q_encoding_fmt, encoding);
- if (sizeof(q_encoding) < i)
- die("Insanely long encoding name %s", encoding);
- memcpy(bp, q_encoding, i);
- bp += i;
- for (i = 0; i < len; i++) {
+ strbuf_add(sb, line, len);
+ return;
+
+needquote:
+ strbuf_addf(sb, "=?%s?q?", encoding);
+ for (i = last = 0; i < len; i++) {
unsigned ch = line[i] & 0xFF;
/*
* We encode ' ' using '=20' even though rfc2047
@@ -518,15 +511,13 @@ static int add_rfc2047(char *buf, const char *line, int len,
* leave the underscore in place.
*/
if (is_rfc2047_special(ch) || ch == ' ') {
- sprintf(bp, "=%02X", ch);
- bp += 3;
+ strbuf_add(sb, line + last, i - last);
+ strbuf_addf(sb, "=%02X", ch);
+ last = i + 1;
}
- else
- *bp++ = ch;
}
- memcpy(bp, "?=", 2);
- bp += 2;
- return bp - buf;
+ strbuf_add(sb, line + last, len - last);
+ strbuf_addstr(sb, "?=");
}
static unsigned long bound_rfc2047(unsigned long len, const char *encoding)
@@ -537,21 +528,21 @@ static unsigned long bound_rfc2047(unsigned long len, const char *encoding)
return len * 3 + elen + 100;
}
-static int add_user_info(const char *what, enum cmit_fmt fmt, char *buf,
+static void add_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
const char *line, enum date_mode dmode,
const char *encoding)
{
char *date;
int namelen;
unsigned long time;
- int tz, ret;
+ int tz;
const char *filler = " ";
if (fmt == CMIT_FMT_ONELINE)
- return 0;
+ return;
date = strchr(line, '>');
if (!date)
- return 0;
+ return;
namelen = ++date - line;
time = strtoul(date, &date, 10);
tz = strtol(date, NULL, 10);
@@ -560,42 +551,35 @@ static int add_user_info(const char *what, enum cmit_fmt fmt, char *buf,
char *name_tail = strchr(line, '<');
int display_name_length;
if (!name_tail)
- return 0;
+ return;
while (line < name_tail && isspace(name_tail[-1]))
name_tail--;
display_name_length = name_tail - line;
filler = "";
- strcpy(buf, "From: ");
- ret = strlen(buf);
- ret += add_rfc2047(buf + ret, line, display_name_length,
- encoding);
- memcpy(buf + ret, name_tail, namelen - display_name_length);
- ret += namelen - display_name_length;
- buf[ret++] = '\n';
+ strbuf_addstr(sb, "From: ");
+ add_rfc2047(sb, line, display_name_length, encoding);
+ strbuf_add(sb, name_tail, namelen - display_name_length);
+ strbuf_addch(sb, '\n');
}
else {
- ret = sprintf(buf, "%s: %.*s%.*s\n", what,
+ strbuf_addf(sb, "%s: %.*s%.*s\n", what,
(fmt == CMIT_FMT_FULLER) ? 4 : 0,
filler, namelen, line);
}
switch (fmt) {
case CMIT_FMT_MEDIUM:
- ret += sprintf(buf + ret, "Date: %s\n",
- show_date(time, tz, dmode));
+ strbuf_addf(sb, "Date: %s\n", show_date(time, tz, dmode));
break;
case CMIT_FMT_EMAIL:
- ret += sprintf(buf + ret, "Date: %s\n",
- show_date(time, tz, DATE_RFC2822));
+ strbuf_addf(sb, "Date: %s\n", show_date(time, tz, DATE_RFC2822));
break;
case CMIT_FMT_FULLER:
- ret += sprintf(buf + ret, "%sDate: %s\n", what,
- show_date(time, tz, dmode));
+ strbuf_addf(sb, "%sDate: %s\n", what, show_date(time, tz, dmode));
break;
default:
/* notin' */
break;
}
- return ret;
}
static int is_empty_line(const char *line, int *len_p)
@@ -607,16 +591,16 @@ static int is_empty_line(const char *line, int *len_p)
return !len;
}
-static int add_merge_info(enum cmit_fmt fmt, char *buf, const struct commit *commit, int abbrev)
+static void add_merge_info(enum cmit_fmt fmt, struct strbuf *sb,
+ const struct commit *commit, int abbrev)
{
struct commit_list *parent = commit->parents;
- int offset;
if ((fmt == CMIT_FMT_ONELINE) || (fmt == CMIT_FMT_EMAIL) ||
!parent || !parent->next)
- return 0;
+ return;
- offset = sprintf(buf, "Merge:");
+ strbuf_addstr(sb, "Merge:");
while (parent) {
struct commit *p = parent->item;
@@ -629,10 +613,9 @@ static int add_merge_info(enum cmit_fmt fmt, char *buf, const struct commit *com
dots = (abbrev && strlen(hex) != 40) ? "..." : "";
parent = parent->next;
- offset += sprintf(buf + offset, " %s%s", hex, dots);
+ strbuf_addf(sb, " %s%s", hex, dots);
}
- buf[offset++] = '\n';
- return offset;
+ strbuf_addch(sb, '\n');
}
static char *get_header(const struct commit *commit, const char *key)
@@ -787,8 +770,8 @@ static void fill_person(struct interp *table, const char *msg, int len)
interp_set_entry(table, 6, show_date(date, tz, DATE_ISO8601));
}
-long format_commit_message(const struct commit *commit, const void *format,
- char **buf_p, unsigned long *space_p)
+void format_commit_message(const struct commit *commit,
+ const void *format, struct strbuf *sb)
{
struct interp table[] = {
{ "%H" }, /* commit hash */
@@ -841,6 +824,7 @@ long format_commit_message(const struct commit *commit, const void *format,
};
struct commit_list *p;
char parents[1024];
+ unsigned long len;
int i;
enum { HEADER, SUBJECT, BODY } state;
const char *msg = commit->buffer;
@@ -921,20 +905,15 @@ long format_commit_message(const struct commit *commit, const void *format,
if (!table[i].value)
interp_set_entry(table, i, "<unknown>");
- do {
- char *buf = *buf_p;
- unsigned long len;
-
- len = interpolate(buf, *space_p, format,
- table, ARRAY_SIZE(table));
- if (len < *space_p)
- break;
- ALLOC_GROW(buf, len + 1, *space_p);
- *buf_p = buf;
- } while (1);
+ len = interpolate(sb->buf + sb->len, strbuf_avail(sb),
+ format, table, ARRAY_SIZE(table));
+ if (len > strbuf_avail(sb)) {
+ strbuf_grow(sb, len);
+ interpolate(sb->buf + sb->len, strbuf_avail(sb) + 1,
+ format, table, ARRAY_SIZE(table));
+ }
+ strbuf_setlen(sb, sb->len + len);
interp_clear_table(table, ARRAY_SIZE(table));
-
- return strlen(*buf_p);
}
static void pp_header(enum cmit_fmt fmt,
@@ -943,34 +922,24 @@ static void pp_header(enum cmit_fmt fmt,
const char *encoding,
const struct commit *commit,
const char **msg_p,
- unsigned long *len_p,
- unsigned long *ofs_p,
- char **buf_p,
- unsigned long *space_p)
+ struct strbuf *sb)
{
int parents_shown = 0;
for (;;) {
const char *line = *msg_p;
- char *dst;
- int linelen = get_one_line(*msg_p, *len_p);
- unsigned long len;
+ int linelen = get_one_line(*msg_p);
if (!linelen)
return;
*msg_p += linelen;
- *len_p -= linelen;
if (linelen == 1)
/* End of header */
return;
- ALLOC_GROW(*buf_p, linelen + *ofs_p + 20, *space_p);
- dst = *buf_p + *ofs_p;
-
if (fmt == CMIT_FMT_RAW) {
- memcpy(dst, line, linelen);
- *ofs_p += linelen;
+ strbuf_add(sb, line, linelen);
continue;
}
@@ -988,10 +957,8 @@ static void pp_header(enum cmit_fmt fmt,
parent = parent->next, num++)
;
/* with enough slop */
- num = *ofs_p + num * 50 + 20;
- ALLOC_GROW(*buf_p, num, *space_p);
- dst = *buf_p + *ofs_p;
- *ofs_p += add_merge_info(fmt, dst, commit, abbrev);
+ strbuf_grow(sb, num * 50 + 20);
+ add_merge_info(fmt, sb, commit, abbrev);
parents_shown = 1;
}
@@ -1001,129 +968,100 @@ static void pp_header(enum cmit_fmt fmt,
* FULLER shows both authors and dates.
*/
if (!memcmp(line, "author ", 7)) {
- len = linelen;
+ unsigned long len = linelen;
if (fmt == CMIT_FMT_EMAIL)
len = bound_rfc2047(linelen, encoding);
- ALLOC_GROW(*buf_p, *ofs_p + len + 80, *space_p);
- dst = *buf_p + *ofs_p;
- *ofs_p += add_user_info("Author", fmt, dst,
- line + 7, dmode, encoding);
+ strbuf_grow(sb, len + 80);
+ add_user_info("Author", fmt, sb, line + 7, dmode, encoding);
}
if (!memcmp(line, "committer ", 10) &&
(fmt == CMIT_FMT_FULL || fmt == CMIT_FMT_FULLER)) {
- len = linelen;
+ unsigned long len = linelen;
if (fmt == CMIT_FMT_EMAIL)
len = bound_rfc2047(linelen, encoding);
- ALLOC_GROW(*buf_p, *ofs_p + len + 80, *space_p);
- dst = *buf_p + *ofs_p;
- *ofs_p += add_user_info("Commit", fmt, dst,
- line + 10, dmode, encoding);
+ strbuf_grow(sb, len + 80);
+ add_user_info("Commit", fmt, sb, line + 10, dmode, encoding);
}
}
}
static void pp_title_line(enum cmit_fmt fmt,
const char **msg_p,
- unsigned long *len_p,
- unsigned long *ofs_p,
- char **buf_p,
- unsigned long *space_p,
- int indent,
+ struct strbuf *sb,
const char *subject,
const char *after_subject,
const char *encoding,
int plain_non_ascii)
{
- char *title;
- unsigned long title_alloc, title_len;
+ struct strbuf title;
unsigned long len;
- title_len = 0;
- title_alloc = 80;
- title = xmalloc(title_alloc);
+ strbuf_init(&title);
+ strbuf_grow(&title, 80);
+
for (;;) {
const char *line = *msg_p;
- int linelen = get_one_line(line, *len_p);
- *msg_p += linelen;
- *len_p -= linelen;
+ int linelen = get_one_line(line);
+ *msg_p += linelen;
if (!linelen || is_empty_line(line, &linelen))
break;
- if (title_alloc <= title_len + linelen + 2) {
- title_alloc = title_len + linelen + 80;
- title = xrealloc(title, title_alloc);
- }
- len = 0;
- if (title_len) {
+ strbuf_grow(&title, linelen + 2);
+ if (title.len) {
if (fmt == CMIT_FMT_EMAIL) {
- len++;
- title[title_len++] = '\n';
+ strbuf_addch(&title, '\n');
}
- len++;
- title[title_len++] = ' ';
+ strbuf_addch(&title, ' ');
}
- memcpy(title + title_len, line, linelen);
- title_len += linelen;
+ strbuf_add(&title, line, linelen);
}
/* Enough slop for the MIME header and rfc2047 */
- len = bound_rfc2047(title_len, encoding)+ 1000;
+ len = bound_rfc2047(title.len, encoding) + 1000;
if (subject)
len += strlen(subject);
if (after_subject)
len += strlen(after_subject);
if (encoding)
len += strlen(encoding);
- ALLOC_GROW(*buf_p, title_len + *ofs_p + len, *space_p);
+ strbuf_grow(sb, title.len + len);
if (subject) {
- len = strlen(subject);
- memcpy(*buf_p + *ofs_p, subject, len);
- *ofs_p += len;
- *ofs_p += add_rfc2047(*buf_p + *ofs_p,
- title, title_len, encoding);
+ strbuf_addstr(sb, subject);
+ add_rfc2047(sb, title.buf, title.len, encoding);
} else {
- memcpy(*buf_p + *ofs_p, title, title_len);
- *ofs_p += title_len;
+ strbuf_addbuf(sb, &title);
}
- (*buf_p)[(*ofs_p)++] = '\n';
+ strbuf_addch(sb, '\n');
+
if (plain_non_ascii) {
const char *header_fmt =
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=%s\n"
"Content-Transfer-Encoding: 8bit\n";
- *ofs_p += snprintf(*buf_p + *ofs_p,
- *space_p - *ofs_p,
- header_fmt, encoding);
+ strbuf_addf(sb, header_fmt, encoding);
}
if (after_subject) {
- len = strlen(after_subject);
- memcpy(*buf_p + *ofs_p, after_subject, len);
- *ofs_p += len;
+ strbuf_addstr(sb, after_subject);
}
- free(title);
if (fmt == CMIT_FMT_EMAIL) {
- ALLOC_GROW(*buf_p, *ofs_p + 20, *space_p);
- (*buf_p)[(*ofs_p)++] = '\n';
+ strbuf_addch(sb, '\n');
}
+ strbuf_release(&title);
}
static void pp_remainder(enum cmit_fmt fmt,
const char **msg_p,
- unsigned long *len_p,
- unsigned long *ofs_p,
- char **buf_p,
- unsigned long *space_p,
+ struct strbuf *sb,
int indent)
{
int first = 1;
for (;;) {
const char *line = *msg_p;
- int linelen = get_one_line(line, *len_p);
+ int linelen = get_one_line(line);
*msg_p += linelen;
- *len_p -= linelen;
if (!linelen)
break;
@@ -1136,36 +1074,30 @@ static void pp_remainder(enum cmit_fmt fmt,
}
first = 0;
- ALLOC_GROW(*buf_p, *ofs_p + linelen + indent + 20, *space_p);
+ strbuf_grow(sb, linelen + indent + 20);
if (indent) {
- memset(*buf_p + *ofs_p, ' ', indent);
- *ofs_p += indent;
+ memset(sb->buf + sb->len, ' ', indent);
+ strbuf_setlen(sb, sb->len + indent);
}
- memcpy(*buf_p + *ofs_p, line, linelen);
- *ofs_p += linelen;
- (*buf_p)[(*ofs_p)++] = '\n';
+ strbuf_add(sb, line, linelen);
+ strbuf_addch(sb, '\n');
}
}
-unsigned long pretty_print_commit(enum cmit_fmt fmt,
- const struct commit *commit,
- unsigned long len,
- char **buf_p, unsigned long *space_p,
- int abbrev, const char *subject,
- const char *after_subject,
+void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
+ struct strbuf *sb, int abbrev,
+ const char *subject, const char *after_subject,
enum date_mode dmode)
{
- unsigned long offset = 0;
unsigned long beginning_of_body;
int indent = 4;
const char *msg = commit->buffer;
int plain_non_ascii = 0;
char *reencoded;
const char *encoding;
- char *buf;
if (fmt == CMIT_FMT_USERFORMAT)
- return format_commit_message(commit, user_format, buf_p, space_p);
+ return format_commit_message(commit, user_format, sb);
encoding = (git_log_output_encoding
? git_log_output_encoding
@@ -1175,7 +1107,6 @@ unsigned long pretty_print_commit(enum cmit_fmt fmt,
reencoded = logmsg_reencode(commit, encoding);
if (reencoded) {
msg = reencoded;
- len = strlen(reencoded);
}
if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
@@ -1190,14 +1121,13 @@ unsigned long pretty_print_commit(enum cmit_fmt fmt,
if (fmt == CMIT_FMT_EMAIL && !after_subject) {
int i, ch, in_body;
- for (in_body = i = 0; (ch = msg[i]) && i < len; i++) {
+ for (in_body = i = 0; (ch = msg[i]); i++) {
if (!in_body) {
/* author could be non 7-bit ASCII but
* the log may be so; skip over the
* header part first.
*/
- if (ch == '\n' &&
- i + 1 < len && msg[i+1] == '\n')
+ if (ch == '\n' && msg[i+1] == '\n')
in_body = 1;
}
else if (non_ascii(ch)) {
@@ -1207,59 +1137,44 @@ unsigned long pretty_print_commit(enum cmit_fmt fmt,
}
}
- pp_header(fmt, abbrev, dmode, encoding,
- commit, &msg, &len,
- &offset, buf_p, space_p);
+ pp_header(fmt, abbrev, dmode, encoding, commit, &msg, sb);
if (fmt != CMIT_FMT_ONELINE && !subject) {
- ALLOC_GROW(*buf_p, offset + 20, *space_p);
- (*buf_p)[offset++] = '\n';
+ strbuf_addch(sb, '\n');
}
/* Skip excess blank lines at the beginning of body, if any... */
for (;;) {
- int linelen = get_one_line(msg, len);
+ int linelen = get_one_line(msg);
int ll = linelen;
if (!linelen)
break;
if (!is_empty_line(msg, &ll))
break;
msg += linelen;
- len -= linelen;
}
/* These formats treat the title line specially. */
- if (fmt == CMIT_FMT_ONELINE
- || fmt == CMIT_FMT_EMAIL)
- pp_title_line(fmt, &msg, &len, &offset,
- buf_p, space_p, indent,
- subject, after_subject, encoding,
- plain_non_ascii);
-
- beginning_of_body = offset;
- if (fmt != CMIT_FMT_ONELINE)
- pp_remainder(fmt, &msg, &len, &offset,
- buf_p, space_p, indent);
-
- while (offset && isspace((*buf_p)[offset-1]))
- offset--;
+ if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
+ pp_title_line(fmt, &msg, sb, subject,
+ after_subject, encoding, plain_non_ascii);
- ALLOC_GROW(*buf_p, offset + 20, *space_p);
- buf = *buf_p;
+ beginning_of_body = sb->len;
+ if (fmt != CMIT_FMT_ONELINE)
+ pp_remainder(fmt, &msg, sb, indent);
+ strbuf_rtrim(sb);
/* Make sure there is an EOLN for the non-oneline case */
if (fmt != CMIT_FMT_ONELINE)
- buf[offset++] = '\n';
+ strbuf_addch(sb, '\n');
/*
* The caller may append additional body text in e-mail
* format. Make sure we did not strip the blank line
* between the header and the body.
*/
- if (fmt == CMIT_FMT_EMAIL && offset <= beginning_of_body)
- buf[offset++] = '\n';
- buf[offset] = '\0';
+ if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
+ strbuf_addch(sb, '\n');
free(reencoded);
- return offset;
}
struct commit *pop_commit(struct commit_list **stack)
@@ -1338,12 +1253,12 @@ void sort_in_topological_order_fn(struct commit_list ** list, int lifo,
next=next->next;
}
/*
- * find the tips
- *
- * tips are nodes not reachable from any other node in the list
- *
- * the tips serve as a starting set for the work queue.
- */
+ * find the tips
+ *
+ * tips are nodes not reachable from any other node in the list
+ *
+ * the tips serve as a starting set for the work queue.
+ */
next=*list;
insert = &work;
while (next) {
@@ -1370,9 +1285,9 @@ void sort_in_topological_order_fn(struct commit_list ** list, int lifo,
if (pn) {
/*
* parents are only enqueued for emission
- * when all their children have been emitted thereby
- * guaranteeing topological order.
- */
+ * when all their children have been emitted thereby
+ * guaranteeing topological order.
+ */
pn->indegree--;
if (!pn->indegree) {
if (!lifo)
@@ -1384,9 +1299,9 @@ void sort_in_topological_order_fn(struct commit_list ** list, int lifo,
parents=parents->next;
}
/*
- * work_item is a commit all of whose children
- * have already been emitted. we can emit it now.
- */
+ * work_item is a commit all of whose children
+ * have already been emitted. we can emit it now.
+ */
*pptr = work_node->list_item;
pptr = &(*pptr)->next;
*pptr = NULL;
@@ -1482,8 +1397,7 @@ static struct commit_list *merge_bases(struct commit *one, struct commit *two)
}
struct commit_list *get_merge_bases(struct commit *one,
- struct commit *two,
- int cleanup)
+ struct commit *two, int cleanup)
{
struct commit_list *list;
struct commit **rslt;
diff --git a/commit.h b/commit.h
index a8d7661..b779de8 100644
--- a/commit.h
+++ b/commit.h
@@ -3,6 +3,7 @@
#include "object.h"
#include "tree.h"
+#include "strbuf.h"
#include "decorate.h"
struct commit_list {
@@ -61,8 +62,12 @@ enum cmit_fmt {
};
extern enum cmit_fmt get_commit_format(const char *arg);
-extern long format_commit_message(const struct commit *commit, const void *template, char **buf_p, unsigned long *space_p);
-extern unsigned long pretty_print_commit(enum cmit_fmt fmt, const struct commit *, unsigned long len, char **buf_p, unsigned long *space_p, int abbrev, const char *subject, const char *after_subject, enum date_mode dmode);
+extern void format_commit_message(const struct commit *commit,
+ const void *format, struct strbuf *sb);
+extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit*,
+ struct strbuf *,
+ int abbrev, const char *subject,
+ const char *after_subject, enum date_mode);
/** Removes the first commit from a list sorted by date, and adds all
* of its parents.
diff --git a/log-tree.c b/log-tree.c
index a642371..5186939 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -79,25 +79,14 @@ static int detect_any_signoff(char *letter, int size)
return seen_head && seen_name;
}
-static unsigned long append_signoff(char **buf_p, unsigned long *buf_sz_p,
- unsigned long at, const char *signoff)
+static void append_signoff(struct strbuf *sb, const char *signoff)
{
static const char signed_off_by[] = "Signed-off-by: ";
size_t signoff_len = strlen(signoff);
int has_signoff = 0;
char *cp;
- char *buf;
- unsigned long buf_sz;
-
- buf = *buf_p;
- buf_sz = *buf_sz_p;
- if (buf_sz <= at + strlen(signed_off_by) + signoff_len + 3) {
- buf_sz += strlen(signed_off_by) + signoff_len + 3;
- buf = xrealloc(buf, buf_sz);
- *buf_p = buf;
- *buf_sz_p = buf_sz;
- }
- cp = buf;
+
+ cp = sb->buf;
/* First see if we already have the sign-off by the signer */
while ((cp = strstr(cp, signed_off_by))) {
@@ -105,29 +94,25 @@ static unsigned long append_signoff(char **buf_p, unsigned long *buf_sz_p,
has_signoff = 1;
cp += strlen(signed_off_by);
- if (cp + signoff_len >= buf + at)
+ if (cp + signoff_len >= sb->buf + sb->len)
break;
if (strncmp(cp, signoff, signoff_len))
continue;
if (!isspace(cp[signoff_len]))
continue;
/* we already have him */
- return at;
+ return;
}
if (!has_signoff)
- has_signoff = detect_any_signoff(buf, at);
+ has_signoff = detect_any_signoff(sb->buf, sb->len);
if (!has_signoff)
- buf[at++] = '\n';
-
- strcpy(buf + at, signed_off_by);
- at += strlen(signed_off_by);
- strcpy(buf + at, signoff);
- at += signoff_len;
- buf[at++] = '\n';
- buf[at] = 0;
- return at;
+ strbuf_addch(sb, '\n');
+
+ strbuf_addstr(sb, signed_off_by);
+ strbuf_add(sb, signoff, signoff_len);
+ strbuf_addch(sb, '\n');
}
static unsigned int digits_in_number(unsigned int number)
@@ -142,14 +127,12 @@ static unsigned int digits_in_number(unsigned int number)
void show_log(struct rev_info *opt, const char *sep)
{
- char *msgbuf = NULL;
- unsigned long msgbuf_len = 0;
+ struct strbuf msgbuf;
struct log_info *log = opt->loginfo;
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;
const char *subject = NULL, *extra_headers = opt->extra_headers;
opt->loginfo = NULL;
@@ -288,18 +271,17 @@ void show_log(struct rev_info *opt, const char *sep)
/*
* And then the pretty-printed message itself
*/
- len = pretty_print_commit(opt->commit_format, commit, ~0u,
- &msgbuf, &msgbuf_len, abbrev, subject,
- extra_headers, opt->date_mode);
+ strbuf_init(&msgbuf);
+ pretty_print_commit(opt->commit_format, commit, &msgbuf,
+ abbrev, subject, extra_headers, opt->date_mode);
if (opt->add_signoff)
- len = append_signoff(&msgbuf, &msgbuf_len, len,
- opt->add_signoff);
+ append_signoff(&msgbuf, opt->add_signoff);
if (opt->show_log_size)
- printf("log size %i\n", len);
+ printf("log size %i\n", (int)msgbuf.len);
- printf("%s%s%s", msgbuf, extra, sep);
- free(msgbuf);
+ printf("%s%s%s", msgbuf.buf, extra, sep);
+ strbuf_release(&msgbuf);
}
int log_tree_diff_flush(struct rev_info *opt)
--
1.5.3.1
^ permalink raw reply related
* Use more strbufs series [on top of next]
From: Pierre Habouzit @ 2007-09-09 0:04 UTC (permalink / raw)
To: git
Here is a series of patches on top of next, which use strbufs in even
more places. Most notably, it uses it in the commit pretty printer (and
commit message formatter), and it supersedes definitely read_fd
(previously in sha1_file.c).
The latter is not strictly speaking necessary, but strbuf_read and
read_fd do almost the same. The sole difference is that read_fd use
custom reallocation mechanisms (which is bad now that strbufs exists)
though had a different semantics when an error occurs. Though, like for
strbuf_read, read_fd callers either die() or discard the buffer, and
current strbuf_read semantics works with that.
As a result we once again have a nice reduction of the code lines.
$ git diff --shortstat origin/next strbuf.*
2 files changed, 24 insertions(+), 0 deletions(-)
$ git diff --shortstat origin/next ^strbuf.*
16 files changed, 270 insertions(+), 466 deletions(-)
I feel that with the current series commit.c can be simplified
further, but the patch was big enough as is already.
If this series will be rewritten for this or this reason, please note
that I found a memory leak (not a severe one though) in
builtin-archive.c, where the "fmt" pointer is allocated and never freed.
Cheers,
^ permalink raw reply
* [PATCH 5/6] Use strbufs to in read_message (imap-send.c), custom buffer--.
From: Pierre Habouzit @ 2007-09-09 0:04 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <11892962771947-git-send-email-madcoder@debian.org>
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
imap-send.c | 31 +++++++++++--------------------
1 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index a5a0696..d3dc5dd 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -23,6 +23,7 @@
*/
#include "cache.h"
+#include "strbuf.h"
typedef struct store_conf {
char *name;
@@ -1160,28 +1161,18 @@ imap_store_msg( store_t *gctx, msg_data_t *data, int *uid )
static int
read_message( FILE *f, msg_data_t *msg )
{
- int len, r;
+ struct strbuf buf;
- memset( msg, 0, sizeof *msg );
- len = CHUNKSIZE;
- msg->data = xmalloc( len+1 );
- msg->data[0] = 0;
-
- while(!feof( f )) {
- if (msg->len >= len) {
- void *p;
- len += CHUNKSIZE;
- p = xrealloc(msg->data, len+1);
- if (!p)
- break;
- msg->data = p;
- }
- r = fread( &msg->data[msg->len], 1, len - msg->len, f );
- if (r <= 0)
+ memset(msg, 0, sizeof(*msg));
+ strbuf_init(&buf);
+
+ do {
+ if (strbuf_fread(&buf, CHUNKSIZE, f) <= 0)
break;
- msg->len += r;
- }
- msg->data[msg->len] = 0;
+ } while (!feof(f));
+
+ msg->len = buf.len;
+ msg->data = strbuf_detach(&buf);
return msg->len;
}
--
1.5.3.1
^ permalink raw reply related
* [PATCH 2/6] Change semantics of interpolate to work like snprintf.
From: Pierre Habouzit @ 2007-09-09 0:04 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <11892962761617-git-send-email-madcoder@debian.org>
Also fix many off-by-ones and a useless memset.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
commit.c | 9 ++++-----
interpolate.c | 20 ++++++++------------
2 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/commit.c b/commit.c
index 99f65ce..25781cc 100644
--- a/commit.c
+++ b/commit.c
@@ -923,15 +923,14 @@ long format_commit_message(const struct commit *commit, const void *format,
do {
char *buf = *buf_p;
- unsigned long space = *space_p;
+ unsigned long len;
- space = interpolate(buf, space, format,
+ len = interpolate(buf, *space_p, format,
table, ARRAY_SIZE(table));
- if (!space)
+ if (len < *space_p)
break;
- buf = xrealloc(buf, space);
+ ALLOC_GROW(buf, len + 1, *space_p);
*buf_p = buf;
- *space_p = space;
} while (1);
interp_clear_table(table, ARRAY_SIZE(table));
diff --git a/interpolate.c b/interpolate.c
index 0082677..ff4fb10 100644
--- a/interpolate.c
+++ b/interpolate.c
@@ -44,9 +44,8 @@ void interp_clear_table(struct interp *table, int ninterps)
* { "%%", "%"},
* }
*
- * Returns 0 on a successful substitution pass that fits in result,
- * Returns a number of bytes needed to hold the full substituted
- * string otherwise.
+ * Returns the length of the substituted string (not including the final \0).
+ * Like with snprintf, if the result is >= reslen, then it overflowed.
*/
unsigned long interpolate(char *result, unsigned long reslen,
@@ -61,8 +60,6 @@ unsigned long interpolate(char *result, unsigned long reslen,
int i;
char c;
- memset(result, 0, reslen);
-
while ((c = *src)) {
if (c == '%') {
/* Try to match an interpolation string. */
@@ -78,9 +75,9 @@ unsigned long interpolate(char *result, unsigned long reslen,
value = interps[i].value;
valuelen = strlen(value);
- if (newlen + valuelen + 1 < reslen) {
+ if (newlen + valuelen < reslen) {
/* Substitute. */
- strncpy(dest, value, valuelen);
+ memcpy(dest, value, valuelen);
dest += valuelen;
}
newlen += valuelen;
@@ -89,14 +86,13 @@ unsigned long interpolate(char *result, unsigned long reslen,
}
}
/* Straight copy one non-interpolation character. */
- if (newlen + 1 < reslen)
+ if (newlen < reslen)
*dest++ = *src;
src++;
newlen++;
}
- if (newlen + 1 < reslen)
- return 0;
- else
- return newlen + 2;
+ if (reslen > 0)
+ *dest = '\0';
+ return newlen;
}
--
1.5.3.1
^ permalink raw reply related
* [PATCH 1/6] Add strbuf_rtrim and strbuf_insert.
From: Pierre Habouzit @ 2007-09-09 0:04 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <11892962763548-git-send-email-madcoder@debian.org>
* strbuf_rtrim removes trailing spaces.
* strbuf_insert insert data at a given position.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
strbuf.c | 18 ++++++++++++++++++
strbuf.h | 6 ++++++
2 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 7136de1..2643ce1 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -28,6 +28,24 @@ void strbuf_grow(struct strbuf *sb, size_t extra) {
ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
}
+void strbuf_rtrim(struct strbuf *sb)
+{
+ while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
+ sb->len--;
+ sb->buf[sb->len] = '\0';
+}
+
+void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len) {
+ strbuf_grow(sb, len);
+ if (pos >= sb->len) {
+ sb->len = pos;
+ } else {
+ memmove(sb->buf + pos + len, sb->buf + pos, sb->len - pos);
+ }
+ memcpy(sb->buf + pos, data, len);
+ strbuf_setlen(sb, sb->len + len);
+}
+
void strbuf_add(struct strbuf *sb, const void *data, size_t len) {
strbuf_grow(sb, len);
memcpy(sb->buf + sb->len, data, len);
diff --git a/strbuf.h b/strbuf.h
index b40dc99..b90cbdf 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -68,6 +68,9 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len) {
extern void strbuf_grow(struct strbuf *, size_t);
+/*----- content related -----*/
+extern void strbuf_rtrim(struct strbuf *);
+
/*----- add data in your buffer -----*/
static inline void strbuf_addch(struct strbuf *sb, int c) {
strbuf_grow(sb, 1);
@@ -75,6 +78,9 @@ static inline void strbuf_addch(struct strbuf *sb, int c) {
sb->buf[sb->len] = '\0';
}
+/* inserts after pos, or appends if pos >= sb->len */
+extern void strbuf_insert(struct strbuf *, size_t pos, const void *, size_t);
+
extern void strbuf_add(struct strbuf *, const void *, size_t);
static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
strbuf_add(sb, s, strlen(s));
--
1.5.3.1
^ permalink raw reply related
* [PATCH 4/6] Use strbuf_read in builtin-fetch-tool.c.
From: Pierre Habouzit @ 2007-09-09 0:04 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <1189296276114-git-send-email-madcoder@debian.org>
xrealloc.use --;
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-fetch--tool.c | 23 ++++++-----------------
1 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 24c7e6f..0dd742f 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -2,27 +2,16 @@
#include "cache.h"
#include "refs.h"
#include "commit.h"
-
-#define CHUNK_SIZE 1024
+#include "strbuf.h"
static char *get_stdin(void)
{
- size_t offset = 0;
- char *data = xmalloc(CHUNK_SIZE);
-
- while (1) {
- ssize_t cnt = xread(0, data + offset, CHUNK_SIZE);
- if (cnt < 0)
- die("error reading standard input: %s",
- strerror(errno));
- if (cnt == 0) {
- data[offset] = 0;
- break;
- }
- offset += cnt;
- data = xrealloc(data, offset + CHUNK_SIZE);
+ struct strbuf buf;
+ strbuf_init(&buf);
+ if (strbuf_read(&buf, 0) < 0) {
+ die("error reading standard input: %s", strerror(errno));
}
- return data;
+ return strbuf_detach(&buf);
}
static void show_new(enum object_type type, unsigned char *sha1_new)
--
1.5.3.1
^ permalink raw reply related
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Andreas Ericsson @ 2007-09-08 23:50 UTC (permalink / raw)
To: Walter Bright; +Cc: git
In-Reply-To: <fbsbul$dg0$1@sea.gmane.org>
Walter Bright wrote:
> David Kastrup wrote:
>> Again, C won't keep you from shooting yourself in the foot.
>
> Right, it won't. A good systems language should do what it can to
> prevent the programmer from *inadvertently* shooting himself in the
> foot, while allowing him to *deliberately* shoot himself in the foot.
>
No, a good systems language should do exactly what it's told. Supporting
tools should tell the programmer if he's risking shooting himself in the
foot.
>
>> You can tell C compilers to
>> check all array accesses, but that is a performance issue.
>
> Runtime checking of arrays in D is a performance issue too, so it is
> selectable via a command line switch.
Same as in C then.
> But more importantly,
>
> 2) For dynamically sized arrays, the dimension of the array is carried
> with the array, so loops automatically loop the correct number of times.
> No runtime check is necessary, and it's easier for the code reviewer to
> visually check the code for correctness.
>
But this introduces handy but, strictly speaking, unnecessary overhead as
well, meaning, in short; 'D is slower than C, but easier to write code in'.
So in essence, it's a bit like Python, but a teensy bit faster and a lot
easier to shoot yourself in the foot with.
What was the niche you were going for when you thought up D? It can't have
been systems programming, because *any* extra baggage is baggage one would
like to get rid of. If it was application programming I fail to see how one
more language would help, as there will be portability problems galore and
it's still considerably slower to develop in than fe Python, while at the
same time being considerably easier to mess up in.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH 1/3] Add strbuf_rtrim (to remove trailing spaces).
From: Pierre Habouzit @ 2007-09-08 23:44 UTC (permalink / raw)
To: René Scharfe, git
In-Reply-To: <20070908225353.GB13385@artemis.corp>
[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]
On Sat, Sep 08, 2007 at 10:53:53PM +0000, Pierre Habouzit wrote:
> On sam, sep 08, 2007 at 04:18:20 +0000, René Scharfe wrote:
> > Pierre Habouzit schrieb:
> > > diff --git a/strbuf.c b/strbuf.c
> > > index acc7fc8..565c343 100644
> > > --- a/strbuf.c
> > > +++ b/strbuf.c
> > > @@ -28,6 +28,13 @@ void strbuf_grow(struct strbuf *sb, size_t extra) {
> > > ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
> > > }
> > >
> > > +void strbuf_rtrim(struct strbuf *sb)
> > > +{
> > > + while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
> > > + sb->len--;
> > > + sb->buf[sb->len] = '\0';
> > > +}
> >
> > Please use tabs instead of spaces to indent, just like you did in your
> > other patches. :)
>
> yeah, good catch.
> I've added some more patches tonight, and have added another function
> in strbufs, and thanks to the awsome git rebase -i, this patch
> integrates this new function as well, so this patch series is obsolete
> already. I wont repost it until I know if I should rebase it on next
> rather than master though.
Okay I'm stupid, Junio already merged my patches in next, doh.
I've seen the conflict btw, and there is a memory leak (on the format)
in builtin-archive.c now... but I'm going to fix it (hopefuly) :)
--
·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: [PATCH] Button added to performs a GUI diff (inline)
From: Johannes Schindelin @ 2007-09-08 23:34 UTC (permalink / raw)
To: pierre.dumuid; +Cc: git
In-Reply-To: <200709081231.26894.pmdumuid@pezdell>
Hi,
On Sat, 8 Sep 2007, Pierre Marc Dumuid wrote:
> From bd43cca7aa88282455b6bbe6e2f9d8134da1029c Mon Sep 17 00:00:00 2001
> From: Pierre Marc Dumuid <pierre.dumuid@adelaide.edu.au>
> Date: Sat, 8 Sep 2007 00:32:10 +0930
> Subject: [PATCH] Button added to performs a GUI diff
Almost! Just make sure that you are sending this patch with the correct
Sender and put the Subject: line as subject of your mail.
And then describe in more detail what the patch does (This commit adds a
button to gitk to easily look at diffs between a commit and its first
parent. It uses the GUI tool selected in merge.tool to visualize the
diff, erroring out if none was specified.)
> + if {[catch {exec git cat-file -p $sha1string^:$fname > .gitk_diffolder} err]
> + || [catch {exec git cat-file -p $sha1string:$fname > .gitk_diffnewer} err]} {
> + error_popup "Invalid file selected for comparison"
> + return
> + }
I think I found a solution for removing the temporary files: man n after.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Alex Riesen @ 2007-09-08 23:25 UTC (permalink / raw)
To: Dmitry Kakurin; +Cc: Johannes Schindelin, Linus Torvalds, Matthieu Moy, Git
In-Reply-To: <a1bbc6950709071732s1f15e5ev28bdfc5c1ab5877b@mail.gmail.com>
Dmitry Kakurin, Sat, Sep 08, 2007 02:32:09 +0200:
> On 9/7/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Thu, 6 Sep 2007, Dmitry Kakurin wrote:
> >
> > > Anyway I don't mean to start a religious C vs. C++ war.
> >
> > You have a very strange way of not meaning to start a C vs. C++ war.
>
> I honestly didn't. I didn't even think it's possible. In the
> environment of mainstream commercial software development the last war
> on this subj was over 8-10 years ago.
It is because the "environment of mainstream commercial software
development" is stuck in "8-10" back from now.
> Even wars like "do we use exceptions/templates/stl" are pretty much
> over. Now days it's "do we use Boost", or "do we use template
> metaprogramming". But even more often it's Java/C# vs. C++.
Now that's a stupid argument to bring up. Commercial software
development is were the most stupid mistakes are done and repeated.
> That's why I was wondering how come C was chosen for Git.
"Just to annoy mainstream commercial software developers" would be a
good reason.
^ 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