Re: [PATCH] Make git-fmt-merge-msg a builtin
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jul 3, 2006, 14:36 UTC
- Message-ID
- <Pine.LNX.4.63.0607031632290.29667@wbgn013.biozentrum.uni-wuerzburg.de>
- In-Reply-To
- <20060703171751.2ed33220.tihirvon@gmail.com>
Hi,
On Mon, 3 Jul 2006, Timo Hirvonen wrote:
Show 22 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> > +struct list {
> > + char **list;
> > + void **payload;
> > + unsigned nr, alloc;
> > +};
>
> How about something like this instead to reduce mallocs to half and
> simplify the code?
>
> struct item {
> char *value;
> void *payload;
> };
>
> struct list {
> struct item *items;
> unsigned int nr, alloc;
> };
>
> (But I realize this isn't performance critical)I had in mind that I want to use path-list instead (which is cooking in the merge-recursive efforts ATM). And there, I would add a flag needs_payload. Opinions?
> > +static void append_to_list(struct list *list, char *value) > > Add void *payload parameter too, would simplify the code.
Okay.
Show 8 quoted lines
> > +static void free_list(struct list *list)
> > +{
> > + int i;
> > +
> > + if (list->alloc == 0)
> > + return;
>
> Unnecessary if nr is 0 too.No. If nr == 0, alloc need not be 0, and if it is not, list and payload are still allocated.
Show 6 quoted lines
> > + for (i = 0; i < list->nr; i++) {
> > + free(list->list[i]);
> > + if (list->payload[i])
> > + free(list->payload[i]);
>
> free(NULL) is safe.Is it? I vaguely remember that I had problems with this on some obscure platform.
Show 5 quoted lines
> > + if (!strncmp(line, "branch ", 7)) {
> > + origin = strdup(line + 7);
> > + append_to_list(&(src_data->branch), origin);
>
> Parenthesis isn't needed.Okay. Wanted to be on the safe side.
> > + head->object.flags |= UNINTERESTING; > > + prepare_revision_walk(rev); > > Spaces..
True. Will fix.
Show 9 quoted lines
> > + if (merge_summary) {
> > + struct commit *head;
> > + struct rev_info rev;
> > +
> > + head = lookup_commit(head_sha1);
> > +parse_object(head->object.sha1);
> > +head = head->parents->item;
>
> Indentation.No. Bug. This was a leftover from my tests (with this, the summary is not done versus HEAD, but HEAD^).
Will fix and resubmit.
Ciao, Dscho