Johannes Schindelin <johannes.schinde...@gmx.de> writes: > It is technically allowed, as per C89, for functions' return type to > be complete structs (i.e. *not* just pointers to structs), but it is > bad practice.
Not necessarily. > This is a very late attempt to contain the damage done by this developer > in 6d297f8 (Status update on merge-recursive in C, 2006-07-08) which > introduced such a return type. > > It will also help the current effort to libify merge-recursive.c, as > it will allow us to return proper error codes later. But this part of the motivation does make sense. Having to pass an extra int &error_code field, only because the return value is already used for something else, is a lot more weird. > Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de> > --- > merge-recursive.c | 93 > ++++++++++++++++++++++++++++++------------------------- > 1 file changed, 50 insertions(+), 43 deletions(-) > > diff --git a/merge-recursive.c b/merge-recursive.c > index c4ece96..d56651c9 100644 > --- a/merge-recursive.c > +++ b/merge-recursive.c > @@ -888,47 +888,47 @@ static int merge_3way(struct merge_options *o, > return merge_status; > } > > -static struct merge_file_info merge_file_1(struct merge_options *o, > +static int merge_file_1(struct merge_options *o, > const struct diff_filespec *one, > const struct diff_filespec *a, > const struct diff_filespec *b, > const char *branch1, > - const char *branch2) > + const char *branch2, > + struct merge_file_info *result) > { > - struct merge_file_info result; > - result.merge = 0; > - result.clean = 1; > + result->merge = 0; > + result->clean = 1; > > if ((S_IFMT & a->mode) != (S_IFMT & b->mode)) { > - result.clean = 0; > + result->clean = 0; > if (S_ISREG(a->mode)) { > - result.mode = a->mode; > - hashcpy(result.sha, a->sha1); > + result->mode = a->mode; > + hashcpy(result->sha, a->sha1); > } else { > - result.mode = b->mode; > - hashcpy(result.sha, b->sha1); > + result->mode = b->mode; > + hashcpy(result->sha, b->sha1); > } > } else { > if (!sha_eq(a->sha1, one->sha1) && !sha_eq(b->sha1, one->sha1)) > - result.merge = 1; > + result->merge = 1; > > /* > * Merge modes > */ > if (a->mode == b->mode || a->mode == one->mode) > - result.mode = b->mode; > + result->mode = b->mode; > else { > - result.mode = a->mode; > + result->mode = a->mode; > if (b->mode != one->mode) { > - result.clean = 0; > - result.merge = 1; > + result->clean = 0; > + result->merge = 1; > } > } > > if (sha_eq(a->sha1, b->sha1) || sha_eq(a->sha1, one->sha1)) > - hashcpy(result.sha, b->sha1); > + hashcpy(result->sha, b->sha1); > else if (sha_eq(b->sha1, one->sha1)) > - hashcpy(result.sha, a->sha1); > + hashcpy(result->sha, a->sha1); > else if (S_ISREG(a->mode)) { > mmbuffer_t result_buf; > int merge_status; > @@ -940,62 +940,63 @@ static struct merge_file_info merge_file_1(struct > merge_options *o, > die(_("Failed to execute internal merge")); > > if (write_sha1_file(result_buf.ptr, result_buf.size, > - blob_type, result.sha)) > + blob_type, result->sha)) > die(_("Unable to add %s to database"), > a->path); > > free(result_buf.ptr); > - result.clean = (merge_status == 0); > + result->clean = (merge_status == 0); > } else if (S_ISGITLINK(a->mode)) { > - result.clean = merge_submodule(result.sha, > + result->clean = merge_submodule(result->sha, > one->path, one->sha1, > a->sha1, b->sha1, > !o->call_depth); > } else if (S_ISLNK(a->mode)) { > - hashcpy(result.sha, a->sha1); > + hashcpy(result->sha, a->sha1); > > if (!sha_eq(a->sha1, b->sha1)) > - result.clean = 0; > + result->clean = 0; > } else > die(_("BUG: unsupported object type in the tree")); > } > > - return result; > + return 0; > } > > -static struct merge_file_info > -merge_file_special_markers(struct merge_options *o, > +static int merge_file_special_markers(struct merge_options *o, > const struct diff_filespec *one, > const struct diff_filespec *a, > const struct diff_filespec *b, > const char *branch1, > const char *filename1, > const char *branch2, > - const char *filename2) > + const char *filename2, > + struct merge_file_info *mfi) > { > char *side1 = NULL; > char *side2 = NULL; > - struct merge_file_info mfi; > + int ret; > > if (filename1) > side1 = xstrfmt("%s:%s", branch1, filename1); > if (filename2) > side2 = xstrfmt("%s:%s", branch2, filename2); > > - mfi = merge_file_1(o, one, a, b, > - side1 ? side1 : branch1, side2 ? side2 : branch2); > + ret = merge_file_1(o, one, a, b, > + side1 ? side1 : branch1, side2 ? side2 : branch2, mfi); > free(side1); > free(side2); > - return mfi; > + return ret; > } > > -static struct merge_file_info merge_file_one(struct merge_options *o, > +static int merge_file_one(struct merge_options *o, > const char *path, > const unsigned char *o_sha, int o_mode, > const unsigned char *a_sha, int a_mode, > const unsigned char *b_sha, int b_mode, > const char *branch1, > - const char *branch2) > + const char *branch2, > + struct merge_file_info *mfi) > { > struct diff_filespec one, a, b; > > @@ -1006,7 +1007,7 @@ static struct merge_file_info merge_file_one(struct > merge_options *o, > a.mode = a_mode; > hashcpy(b.sha1, b_sha); > b.mode = b_mode; > - return merge_file_1(o, &one, &a, &b, branch1, branch2); > + return merge_file_1(o, &one, &a, &b, branch1, branch2, mfi); > } > > static void handle_change_delete(struct merge_options *o, > @@ -1179,11 +1180,14 @@ static void conflict_rename_rename_1to2(struct > merge_options *o, > struct merge_file_info mfi; > struct diff_filespec other; > struct diff_filespec *add; > - mfi = merge_file_one(o, one->path, > + > + if (merge_file_one(o, one->path, > one->sha1, one->mode, > a->sha1, a->mode, > b->sha1, b->mode, > - ci->branch1, ci->branch2); > + ci->branch1, ci->branch2, &mfi) < 0) > + return; > + > /* > * FIXME: For rename/add-source conflicts (if we could detect > * such), this is wrong. We should instead find a unique > @@ -1237,12 +1241,13 @@ static void conflict_rename_rename_2to1(struct > merge_options *o, > remove_file(o, 1, a->path, o->call_depth || > would_lose_untracked(a->path)); > remove_file(o, 1, b->path, o->call_depth || > would_lose_untracked(b->path)); > > - mfi_c1 = merge_file_special_markers(o, a, c1, &ci->ren1_other, > + if (merge_file_special_markers(o, a, c1, &ci->ren1_other, > o->branch1, c1->path, > - o->branch2, ci->ren1_other.path); > - mfi_c2 = merge_file_special_markers(o, b, &ci->ren2_other, c2, > + o->branch2, ci->ren1_other.path, > &mfi_c1) < 0 || > + merge_file_special_markers(o, b, &ci->ren2_other, c2, > o->branch1, ci->ren2_other.path, > - o->branch2, c2->path); > + o->branch2, c2->path, &mfi_c2) < 0) > + return; > > if (o->call_depth) { > /* > @@ -1463,10 +1468,11 @@ static int process_renames(struct merge_options *o, > ren1_dst, branch2); > if (o->call_depth) { > struct merge_file_info mfi; > - mfi = merge_file_one(o, ren1_dst, > null_sha1, 0, > + if (merge_file_one(o, ren1_dst, > null_sha1, 0, > ren1->pair->two->sha1, > ren1->pair->two->mode, > dst_other.sha1, > dst_other.mode, > - branch1, branch2); > + branch1, branch2, > &mfi) < 0) > + return -1; > output(o, 1, _("Adding merged %s"), > ren1_dst); > update_file(o, 0, mfi.sha, mfi.mode, > ren1_dst); > try_merge = 0; > @@ -1622,9 +1628,10 @@ static int merge_content(struct merge_options *o, > if (dir_in_way(path, !o->call_depth)) > df_conflict_remains = 1; > } > - mfi = merge_file_special_markers(o, &one, &a, &b, > + if (merge_file_special_markers(o, &one, &a, &b, > o->branch1, path1, > - o->branch2, path2); > + o->branch2, path2, &mfi) < 0) > + return -1; > > if (mfi.clean && !df_conflict_remains && > sha_eq(mfi.sha, a_sha) && mfi.mode == a_mode) { -- 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