Re: [PATCH 2/2] fast-export: refactor get_tags_and_duplicates()
Junio C Hamano writes: > Felipe Contreras writes: > >> Split into a separate helper function get_commit() so that the part that >> finds the relevant commit, and the part that does something with it >> (handle tag object, etc.) are in different places. >> >> No functional changes. > > Actually, the old code used to use commit unchecked if e->item->type > said it is OBJ_COMMIT but e->item was somehow NULL. The new code > checks ... Nah, I am an idiot. We will segfault where we check e->item->type before reaching that far down the code either way, so this is really a no-op but makes it easier to fix (if we wanted to---I do not think it matters that much). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] fast-export: refactor get_tags_and_duplicates()
Felipe Contreras writes: > Split into a separate helper function get_commit() so that the part that > finds the relevant commit, and the part that does something with it > (handle tag object, etc.) are in different places. > > No functional changes. Actually, the old code used to use commit unchecked if e->item->type said it is OBJ_COMMIT but e->item was somehow NULL. The new code checks this case and skips with a warning(), which I think is an improvement, if not a bugfix (it only makes it easier to diagnose a bug in the code that populates rev_cmdline_entry). Thanks; will queue. > Signed-off-by: Felipe Contreras > --- > builtin/fast-export.c | 68 > --- > 1 file changed, 38 insertions(+), 30 deletions(-) > > diff --git a/builtin/fast-export.c b/builtin/fast-export.c > index 957392c..03e1090 100644 > --- a/builtin/fast-export.c > +++ b/builtin/fast-export.c > @@ -485,9 +485,32 @@ static void handle_tag(const char *name, struct tag *tag) > (int)message_size, (int)message_size, message ? message : ""); > } > > +static struct commit *get_commit(struct rev_cmdline_entry *e, char > *full_name) > +{ > + switch (e->item->type) { > + case OBJ_COMMIT: > + return (struct commit *)e->item; > + case OBJ_TAG: { > + struct tag *tag = (struct tag *)e->item; > + > + /* handle nested tags */ > + while (tag && tag->object.type == OBJ_TAG) { > + parse_object(tag->object.sha1); > + string_list_append(&extra_refs, full_name)->util = tag; > + tag = (struct tag *)tag->tagged; > + } > + if (!tag) > + die("Tag %s points nowhere?", e->name); > + return (struct commit *)tag; > + break; > + } > + default: > + return NULL; > + } > +} > + > static void get_tags_and_duplicates(struct rev_cmdline_info *info) > { > - struct tag *tag; > int i; > > for (i = 0; i < info->nr; i++) { > @@ -502,41 +525,26 @@ static void get_tags_and_duplicates(struct > rev_cmdline_info *info) > if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1) > continue; > > - switch (e->item->type) { > - case OBJ_COMMIT: > - commit = (struct commit *)e->item; > - break; > - case OBJ_TAG: > - tag = (struct tag *)e->item; > - > - /* handle nested tags */ > - while (tag && tag->object.type == OBJ_TAG) { > - parse_object(tag->object.sha1); > - string_list_append(&extra_refs, > full_name)->util = tag; > - tag = (struct tag *)tag->tagged; > - } > - if (!tag) > - die ("Tag %s points nowhere?", e->name); > - switch(tag->object.type) { > - case OBJ_COMMIT: > - commit = (struct commit *)tag; > - break; > - case OBJ_BLOB: > - export_blob(tag->object.sha1); > - continue; > - default: /* OBJ_TAG (nested tags) is already handled */ > - warning("Tag points to object of unexpected > type %s, skipping.", > - typename(tag->object.type)); > - continue; > - } > - break; > - default: > + commit = get_commit(e, full_name); > + if (!commit) { > warning("%s: Unexpected object of type %s, skipping.", > e->name, > typename(e->item->type)); > continue; > } > > + switch(commit->object.type) { > + case OBJ_COMMIT: > + break; > + case OBJ_BLOB: > + export_blob(commit->object.sha1); > + continue; > + default: /* OBJ_TAG (nested tags) is already handled */ > + warning("Tag points to object of unexpected type %s, > skipping.", > + typename(commit->object.type)); > + continue; > + } > + > /* >* This ref will not be updated through a commit, lets make >* sure it gets properly updated eventually. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] fast-export: refactor get_tags_and_duplicates()
Split into a separate helper function get_commit() so that the part that finds the relevant commit, and the part that does something with it (handle tag object, etc.) are in different places. No functional changes. Signed-off-by: Felipe Contreras --- builtin/fast-export.c | 68 --- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index 957392c..03e1090 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -485,9 +485,32 @@ static void handle_tag(const char *name, struct tag *tag) (int)message_size, (int)message_size, message ? message : ""); } +static struct commit *get_commit(struct rev_cmdline_entry *e, char *full_name) +{ + switch (e->item->type) { + case OBJ_COMMIT: + return (struct commit *)e->item; + case OBJ_TAG: { + struct tag *tag = (struct tag *)e->item; + + /* handle nested tags */ + while (tag && tag->object.type == OBJ_TAG) { + parse_object(tag->object.sha1); + string_list_append(&extra_refs, full_name)->util = tag; + tag = (struct tag *)tag->tagged; + } + if (!tag) + die("Tag %s points nowhere?", e->name); + return (struct commit *)tag; + break; + } + default: + return NULL; + } +} + static void get_tags_and_duplicates(struct rev_cmdline_info *info) { - struct tag *tag; int i; for (i = 0; i < info->nr; i++) { @@ -502,41 +525,26 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info) if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1) continue; - switch (e->item->type) { - case OBJ_COMMIT: - commit = (struct commit *)e->item; - break; - case OBJ_TAG: - tag = (struct tag *)e->item; - - /* handle nested tags */ - while (tag && tag->object.type == OBJ_TAG) { - parse_object(tag->object.sha1); - string_list_append(&extra_refs, full_name)->util = tag; - tag = (struct tag *)tag->tagged; - } - if (!tag) - die ("Tag %s points nowhere?", e->name); - switch(tag->object.type) { - case OBJ_COMMIT: - commit = (struct commit *)tag; - break; - case OBJ_BLOB: - export_blob(tag->object.sha1); - continue; - default: /* OBJ_TAG (nested tags) is already handled */ - warning("Tag points to object of unexpected type %s, skipping.", - typename(tag->object.type)); - continue; - } - break; - default: + commit = get_commit(e, full_name); + if (!commit) { warning("%s: Unexpected object of type %s, skipping.", e->name, typename(e->item->type)); continue; } + switch(commit->object.type) { + case OBJ_COMMIT: + break; + case OBJ_BLOB: + export_blob(commit->object.sha1); + continue; + default: /* OBJ_TAG (nested tags) is already handled */ + warning("Tag points to object of unexpected type %s, skipping.", + typename(commit->object.type)); + continue; + } + /* * This ref will not be updated through a commit, lets make * sure it gets properly updated eventually. -- 1.8.4-337-g7358a66-dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html