midx-write: simplify error cases
The write_midx_internal() method uses gotos to jump to a cleanup section to clear memory before returning 'result'. Since these jumps are more common for error conditions, initialize 'result' to -1 and then only set it to 0 before returning with success. There are a couple places where we return with success via a jump. This has the added benefit that the method now returns -1 on error instead of an inconsistent 1 or -1. Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
committed by
Junio C Hamano
parent
1f2bc6be1d
commit
c25651aefd
26
midx-write.c
26
midx-write.c
@@ -1064,7 +1064,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
|
|||||||
int bitmapped_packs_concat_len = 0;
|
int bitmapped_packs_concat_len = 0;
|
||||||
int pack_name_concat_len = 0;
|
int pack_name_concat_len = 0;
|
||||||
int dropped_packs = 0;
|
int dropped_packs = 0;
|
||||||
int result = 0;
|
int result = -1;
|
||||||
const char **keep_hashes = NULL;
|
const char **keep_hashes = NULL;
|
||||||
struct chunkfile *cf;
|
struct chunkfile *cf;
|
||||||
|
|
||||||
@@ -1117,14 +1117,12 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
|
|||||||
error(_("could not load reverse index for MIDX %s"),
|
error(_("could not load reverse index for MIDX %s"),
|
||||||
hash_to_hex_algop(get_midx_checksum(m),
|
hash_to_hex_algop(get_midx_checksum(m),
|
||||||
m->repo->hash_algo));
|
m->repo->hash_algo));
|
||||||
result = 1;
|
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
}
|
||||||
ctx.num_multi_pack_indexes_before++;
|
ctx.num_multi_pack_indexes_before++;
|
||||||
m = m->base_midx;
|
m = m->base_midx;
|
||||||
}
|
}
|
||||||
} else if (ctx.m && fill_packs_from_midx(&ctx)) {
|
} else if (ctx.m && fill_packs_from_midx(&ctx)) {
|
||||||
result = 1;
|
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1160,12 +1158,16 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
|
|||||||
*/
|
*/
|
||||||
if (!want_bitmap)
|
if (!want_bitmap)
|
||||||
clear_midx_files_ext(object_dir, "bitmap", NULL);
|
clear_midx_files_ext(object_dir, "bitmap", NULL);
|
||||||
|
|
||||||
|
result = 0;
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (ctx.incremental && !ctx.nr)
|
if (ctx.incremental && !ctx.nr) {
|
||||||
|
result = 0;
|
||||||
goto cleanup; /* nothing to do */
|
goto cleanup; /* nothing to do */
|
||||||
|
}
|
||||||
|
|
||||||
if (preferred_pack_name) {
|
if (preferred_pack_name) {
|
||||||
ctx.preferred_pack_idx = NO_PREFERRED_PACK;
|
ctx.preferred_pack_idx = NO_PREFERRED_PACK;
|
||||||
@@ -1239,7 +1241,6 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
|
|||||||
if (!preferred->num_objects) {
|
if (!preferred->num_objects) {
|
||||||
error(_("cannot select preferred pack %s with no objects"),
|
error(_("cannot select preferred pack %s with no objects"),
|
||||||
preferred->pack_name);
|
preferred->pack_name);
|
||||||
result = 1;
|
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1278,10 +1279,8 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (missing_drops) {
|
if (missing_drops)
|
||||||
result = 1;
|
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@@ -1327,7 +1326,6 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
|
|||||||
|
|
||||||
if (ctx.nr - dropped_packs == 0) {
|
if (ctx.nr - dropped_packs == 0) {
|
||||||
error(_("no pack files to index."));
|
error(_("no pack files to index."));
|
||||||
result = 1;
|
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1347,14 +1345,12 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
|
|||||||
incr = mks_tempfile_m(midx_name.buf, 0444);
|
incr = mks_tempfile_m(midx_name.buf, 0444);
|
||||||
if (!incr) {
|
if (!incr) {
|
||||||
error(_("unable to create temporary MIDX layer"));
|
error(_("unable to create temporary MIDX layer"));
|
||||||
result = -1;
|
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (adjust_shared_perm(r, get_tempfile_path(incr))) {
|
if (adjust_shared_perm(r, get_tempfile_path(incr))) {
|
||||||
error(_("unable to adjust shared permissions for '%s'"),
|
error(_("unable to adjust shared permissions for '%s'"),
|
||||||
get_tempfile_path(incr));
|
get_tempfile_path(incr));
|
||||||
result = -1;
|
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1432,7 +1428,6 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
|
|||||||
midx_hash, &pdata, commits, commits_nr,
|
midx_hash, &pdata, commits, commits_nr,
|
||||||
flags) < 0) {
|
flags) < 0) {
|
||||||
error(_("could not write multi-pack bitmap"));
|
error(_("could not write multi-pack bitmap"));
|
||||||
result = 1;
|
|
||||||
clear_packing_data(&pdata);
|
clear_packing_data(&pdata);
|
||||||
free(commits);
|
free(commits);
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
@@ -1458,21 +1453,17 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
|
|||||||
|
|
||||||
if (!chainf) {
|
if (!chainf) {
|
||||||
error_errno(_("unable to open multi-pack-index chain file"));
|
error_errno(_("unable to open multi-pack-index chain file"));
|
||||||
result = -1;
|
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (link_midx_to_chain(ctx.base_midx) < 0) {
|
if (link_midx_to_chain(ctx.base_midx) < 0)
|
||||||
result = -1;
|
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
|
||||||
|
|
||||||
get_split_midx_filename_ext(r->hash_algo, &final_midx_name,
|
get_split_midx_filename_ext(r->hash_algo, &final_midx_name,
|
||||||
object_dir, midx_hash, MIDX_EXT_MIDX);
|
object_dir, midx_hash, MIDX_EXT_MIDX);
|
||||||
|
|
||||||
if (rename_tempfile(&incr, final_midx_name.buf) < 0) {
|
if (rename_tempfile(&incr, final_midx_name.buf) < 0) {
|
||||||
error_errno(_("unable to rename new multi-pack-index layer"));
|
error_errno(_("unable to rename new multi-pack-index layer"));
|
||||||
result = -1;
|
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1505,6 +1496,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
|
|||||||
clear_midx_files(r, object_dir, keep_hashes,
|
clear_midx_files(r, object_dir, keep_hashes,
|
||||||
ctx.num_multi_pack_indexes_before + 1,
|
ctx.num_multi_pack_indexes_before + 1,
|
||||||
ctx.incremental);
|
ctx.incremental);
|
||||||
|
result = 0;
|
||||||
|
|
||||||
cleanup:
|
cleanup:
|
||||||
for (size_t i = 0; i < ctx.nr; i++) {
|
for (size_t i = 0; i < ctx.nr; i++) {
|
||||||
|
|||||||
Reference in New Issue
Block a user