On Saturday 07 August 2010 06:03:05 Nguyen Thai Ngoc Duy wrote: > On Thu, Aug 5, 2010 at 9:41 PM, Christian Couder > > <chriscool@xxxxxxxxxxxxx> wrote: > > It looks like parse_commit() is buggy regarding replaced objects. But I > > am not sure how it should be fixed. > > It could be fixed the same way you did with parse_object(): replace > read_sha1_file() with read_sha1_file_repl(). You would also need to > fix parse_tree() and parse_tag(). But.. > > > Anyway if you use parse_object(), then you don't need parse_commit(). So > > if possible you should use parse_object() instead of both > > lookup_commit() and parse_commit(). > > That's how those functions are used. For example, in > traverse_commit_list(), lookup_*() may be called and uninteresting > objects marked UNINTERESTING. Later on in process_{tree,blob,tag}, > parse_* may be called if their content is interesting. > > To me, the fix above will leave a gap when object->sha1 is the > original sha1, until parse_*() is called. It just does not sound good. What do you think about adding a parse_commit_repl() function like the patch below and then using it instead of parse_commit()? ------- >8 --------------------------------------------------- diff --git a/commit.c b/commit.c index 652c1ba..183a735 100644 --- a/commit.c +++ b/commit.c @@ -316,6 +316,50 @@ int parse_commit(struct commit *item) return ret; } +int parse_commit_repl(struct commit **commit) +{ + enum object_type type; + void *buffer; + unsigned long size; + int ret; + const unsigned char *repl; + struct commit *item = *commit; + + if (!item) + return -1; + if (item->object.parsed) + return 0; + buffer = read_sha1_file_repl(item->object.sha1, &type, &size, &repl); + if (!buffer) + return error("Could not read %s", + sha1_to_hex(item->object.sha1)); + + if (item->object.sha1 != repl) { + struct commit *repl_item = lookup_commit(repl); + if (!repl_item) { + free(buffer); + return error("Bad replacement %s for commit %s", + sha1_to_hex(repl), + sha1_to_hex(item->object.sha1)); + } + repl_item->object.flags = item->object.flags; + *commit = item = repl_item; + } + + if (type != OBJ_COMMIT) { + free(buffer); + return error("Object %s not a commit", + sha1_to_hex(item->object.sha1)); + } + ret = parse_commit_buffer(item, buffer, size); + if (save_commit_buffer && !ret) { + item->buffer = buffer; + return 0; + } + free(buffer); + return ret; +} + int find_commit_subject(const char *commit_buffer, const char **subject) { const char *eol; -------- 8< -------------------------------------------------- Thanks, Christian. -- 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