@matheusbsbs / matheus-bagliano / commits / c0049ca0d7

diff: drop useless return values in git-diff helpers

Since git-diff has many diff modes, it dispatches to many helpers to perform each one. But every helper simply returns "0", as it exits directly if there are serious errors (and options like --exit-code are handled afterwards). So let's get rid of these useless return values, which makes the code flow more clear. There's very little chance that we'd later want to propagate errors instead of dying immediately. These are all static-local helpers for the git-diff program implementing its various modes. More "lib-ified" code would directly call the underlying functions. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Aug 21, 2023 at 16:19 UTC c0049ca0d7d4afb2d71ac76eba45d693facfc1d3

1 file changed +28 -34

builtin/diff.c

+28 -34

@@ -77,9 +77,9 @@ static void stuff_change(struct diff_options *opt,

77 diff_queue(&diff_queued_diff, one, two);

78 }

80 -static int builtin_diff_b_f(struct rev_info *revs,

81 - int argc, const char **argv UNUSED,

82 - struct object_array_entry **blob)

80 +static void builtin_diff_b_f(struct rev_info *revs,

81 + int argc, const char **argv UNUSED,

82 + struct object_array_entry **blob)

83 {

84 /* Blob vs file in the working tree*/

85 struct stat st;

@@ -109,12 +109,11 @@ static int builtin_diff_b_f(struct rev_info *revs,

109 path);

110 diffcore_std(&revs->diffopt);

111 diff_flush(&revs->diffopt);

112 - return 0;

112 }

113

115 -static int builtin_diff_blobs(struct rev_info *revs,

116 - int argc, const char **argv UNUSED,

117 - struct object_array_entry **blob)

114 +static void builtin_diff_blobs(struct rev_info *revs,

115 + int argc, const char **argv UNUSED,

116 + struct object_array_entry **blob)

117 {

118 const unsigned mode = canon_mode(S_IFREG | 0644);

119

@@ -134,11 +133,10 @@ static int builtin_diff_blobs(struct rev_info *revs,

133 blob_path(blob[0]), blob_path(blob[1]));

134 diffcore_std(&revs->diffopt);

135 diff_flush(&revs->diffopt);

137 - return 0;

136 }

137

140 -static int builtin_diff_index(struct rev_info *revs,

141 - int argc, const char **argv)

138 +static void builtin_diff_index(struct rev_info *revs,

139 + int argc, const char **argv)

140 {

141 unsigned int option = 0;

142 while (1 < argc) {

@@ -169,13 +167,12 @@ static int builtin_diff_index(struct rev_info *revs,

167 die_errno("repo_read_cache");

168 }

169 run_diff_index(revs, option);

172 - return 0;

170 }

171

175 -static int builtin_diff_tree(struct rev_info *revs,

176 - int argc, const char **argv,

177 - struct object_array_entry *ent0,

178 - struct object_array_entry *ent1)

172 +static void builtin_diff_tree(struct rev_info *revs,

173 + int argc, const char **argv,

174 + struct object_array_entry *ent0,

175 + struct object_array_entry *ent1)

176 {

177 const struct object_id *(oid[2]);

178 struct object_id mb_oid;

@@ -208,13 +205,12 @@ static int builtin_diff_tree(struct rev_info *revs,

205 }

206 diff_tree_oid(oid[0], oid[1], "", &revs->diffopt);

207 log_tree_diff_flush(revs);

211 - return 0;

208 }

209

214 -static int builtin_diff_combined(struct rev_info *revs,

215 - int argc, const char **argv UNUSED,

216 - struct object_array_entry *ent,

217 - int ents, int first_non_parent)

210 +static void builtin_diff_combined(struct rev_info *revs,

211 + int argc, const char **argv UNUSED,

212 + struct object_array_entry *ent,

213 + int ents, int first_non_parent)

214 {

215 struct oid_array parents = OID_ARRAY_INIT;

216 int i;

@@ -235,7 +231,6 @@ static int builtin_diff_combined(struct rev_info *revs,

231 }

232 diff_tree_combined(&ent[first_non_parent].item->oid, &parents, revs);

233 oid_array_clear(&parents);

238 - return 0;

234 }

235

236 static void refresh_index_quietly(void)

@@ -253,7 +248,7 @@ static void refresh_index_quietly(void)

248 repo_update_index_if_able(the_repository, &lock_file);

249 }

250

256 -static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv)

251 +static void builtin_diff_files(struct rev_info *revs, int argc, const char **argv)

252 {

253 unsigned int options = 0;

254

@@ -291,7 +286,6 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv

286 die_errno("repo_read_index_preload");

287 }

288 run_diff_files(revs, options);

294 - return 0;

289 }

290

291 struct symdiff {

@@ -405,7 +399,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)

399 int blobs = 0, paths = 0;

400 struct object_array_entry *blob[2];

401 int nongit = 0, no_index = 0;

408 - int result = 0;

402 + int result;

403 struct symdiff sdiff;

404

405 /*

@@ -584,17 +578,17 @@ int cmd_diff(int argc, const char **argv, const char *prefix)

578 if (!ent.nr) {

579 switch (blobs) {

580 case 0:

587 - result = builtin_diff_files(&rev, argc, argv);

581 + builtin_diff_files(&rev, argc, argv);

582 break;

583 case 1:

584 if (paths != 1)

585 usage(builtin_diff_usage);

592 - result = builtin_diff_b_f(&rev, argc, argv, blob);

586 + builtin_diff_b_f(&rev, argc, argv, blob);

587 break;

588 case 2:

589 if (paths)

590 usage(builtin_diff_usage);

597 - result = builtin_diff_blobs(&rev, argc, argv, blob);

591 + builtin_diff_blobs(&rev, argc, argv, blob);

592 break;

593 default:

594 usage(builtin_diff_usage);

@@ -603,18 +597,18 @@ int cmd_diff(int argc, const char **argv, const char *prefix)

597 else if (blobs)

598 usage(builtin_diff_usage);

599 else if (ent.nr == 1)

606 - result = builtin_diff_index(&rev, argc, argv);

600 + builtin_diff_index(&rev, argc, argv);

601 else if (ent.nr == 2) {

602 if (sdiff.warn)

603 warning(_("%s...%s: multiple merge bases, using %s"),

604 sdiff.left, sdiff.right, sdiff.base);

611 - result = builtin_diff_tree(&rev, argc, argv,

612 - &ent.objects[0], &ent.objects[1]);

605 + builtin_diff_tree(&rev, argc, argv,

606 + &ent.objects[0], &ent.objects[1]);

607 } else

614 - result = builtin_diff_combined(&rev, argc, argv,

615 - ent.objects, ent.nr,

616 - first_non_parent);

617 - result = diff_result_code(&rev.diffopt, result);

608 + builtin_diff_combined(&rev, argc, argv,

609 + ent.objects, ent.nr,

610 + first_non_parent);

611 + result = diff_result_code(&rev.diffopt, 0);

612 if (1 < rev.diffopt.skip_stat_unmatch)

613 refresh_index_quietly();

614 release_revisions(&rev);