Hi,
On Mon, 5 Nov 2007, Ren? Scharfe wrote:
> Junio C Hamano schrieb:
> > Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> >
> >> Unfortunately, we cannot reuse the result of that function, which
> >> would be cleaner: there are more users than just git log. Most
> >> notably, git-archive with "$Format:...$" substitution.
> >
> > That makes sense.
> >
> >
> >> diff --git a/pretty.c b/pretty.c
> >> index 490cede..241e91c 100644
> >> --- a/pretty.c
> >> +++ b/pretty.c
> >> @@ -393,6 +393,7 @@ void format_commit_message(const struct commit *commit,
> >> int i;
> >> enum { HEADER, SUBJECT, BODY } state;
> >> const char *msg = commit->buffer;
> >> + char *active = interp_find_active(format, table, ARRAY_SIZE(table));
> >> ...
> >> + if (active[IHASH])
> >> + interp_set_entry(table, IHASH,
> >> + sha1_to_hex(commit->object.sha1));
> >> + if (active[IHASH_ABBREV])
> >> + interp_set_entry(table, IHASH_ABBREV,
> >> find_unique_abbrev(commit->object.sha1,
> >> DEFAULT_ABBREV));
> >
> > Instead of allocating a separate array and freeing at the end,
> > wouldn't it make more sense to have a bitfield that records what
> > is used by the format string inside the array elements?
>
> How about (ab)using the value field? Let interp_find_active() mark
> unneeded entries with NULL, and the rest with some cookie. All table
> entries with non-NULL values need to be initialized. interp_set_entry()
> needs to be aware of this cookie, as it mustn't free() it. The cookie
> could be the address of a static char* in interpolate.c.
Yeah, something like this on top of my earlier patch (and obviously the
corresponding change from "if (active[IHASH])" to
"if (table[IHASH].value)"):
---
interpolate.c | 10 ++++------
interpolate.h | 2 +-
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/interpolate.c b/interpolate.c
index 80eeb36..05a22e1 100644
--- a/interpolate.c
+++ b/interpolate.c
@@ -5,13 +5,14 @@
#include "git-compat-util.h"
#include "interpolate.h"
+static const char *empty_value = "";
void interp_set_entry(struct interp *table, int slot, const char *value)
{
char *oldval = table[slot].value;
char *newval = NULL;
- if (oldval)
+ if (oldval && oldval != empty_value)
free(oldval);
if (value)
@@ -103,10 +104,9 @@ unsigned long interpolate(char *result, unsigned long reslen,
return newlen;
}
-char *interp_find_active(const char *orig,
+void interp_find_active(const char *orig,
const struct interp *interps, int ninterps)
{
- char *result = xcalloc(1, ninterps);
char c;
int i;
@@ -115,10 +115,8 @@ char *interp_find_active(const char *orig,
/* Try to match an interpolation string. */
for (i = 0; i < ninterps; i++)
if (!prefixcmp(orig, interps[i].name + 1)) {
- result[i] = 1;
+ interps[i].value = empty_value;
orig += strlen(interps[i].name + 1);
break;
}
-
- return result;
}
diff --git a/interpolate.h b/interpolate.h
index 2d197c5..19b7ebe 100644
--- a/interpolate.h
+++ b/interpolate.h
@@ -22,7 +22,7 @@ extern void interp_clear_table(struct interp *table, int ninterps);
extern unsigned long interpolate(char *result, unsigned long reslen,
const char *orig,
const struct interp *interps, int ninterps);
-extern char *interp_find_active(const char *orig,
+extern void interp_find_active(const char *orig,
const struct interp *interps, int ninterps);
#endif /* INTERPOLATE_H */
Hmm?
Ciao,
Dscho
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
