Johannes Schindelin <johannes.schindelin@xxxxxx> writes: > @@ -743,6 +741,8 @@ static int update_file_flags(struct merge_options *o, > int update_cache, > int update_wd) > ... > + ret = error_errno(_("do not know what to do with %06o %s '%s'"), > + mode, sha1_to_hex(sha), path); > free_buf: OK, with a few more users of this label, it no longer looks so strange to me to have this label here. At least, match the indentation level to the existing one we see below, though? > free(buf); > } > update_index: > - if (update_cache) > + if (!ret && update_cache) > add_cacheinfo(mode, sha, path, 0, update_wd, ADD_CACHE_OK_TO_ADD); Unlike my reaction to "if (!ret) do this" in an earlier patch, I do think this change is sensible. We wouldn't have reached here in the original code if we saw errors, and some of the variable used to call add_cacheinfo() at this point may be garbage when ret is non-zero, i.e. we know we earlier saw an error. > - return 0; > + return ret; > } > ... > if ((merge_status < 0) || !result_buf.ptr) > - die(_("Failed to execute internal merge")); > + ret = error(_("Failed to execute internal merge")); > > - if (write_sha1_file(result_buf.ptr, result_buf.size, > + if (!ret && write_sha1_file(result_buf.ptr, result_buf.size, > blob_type, result->sha)) Likewise. > @@ -1861,7 +1867,7 @@ static int process_entry(struct merge_options *o, > */ > remove_file(o, 1, path, !a_mode); > } else > - die(_("Fatal merge failure, shouldn't happen.")); > + return error(_("Fatal merge failure, shouldn't happen.")); Isn't this BUG()? -- 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