Re: [PATCH 6/9] index-pack: refactor renaming in final()
flat view
From: Junio C Hamano <hidden>
Date: 2021-09-09 19:45:08
Taylor Blau [off-list ref] writes:
But since we've got "rev" as well now, let's do the renaming via a helper, this is both a net decrease in lines, and improves the readability,...
xyz_ may be cute, but is distracting. I do not think it loses any information if we used final_name, current_name, etc.; it may make the result even easier to read.
quoted hunk
+static void rename_tmp_packfile(const char **final_xyz_name, + const char *curr_xyz_name, + struct strbuf *xyz_name, unsigned char *hash, + const char *ext, int else_chmod_if) +{ + if (*final_xyz_name != curr_xyz_name) { + if (!*final_xyz_name) + *final_xyz_name = odb_pack_name(xyz_name, hash, ext); + if (finalize_object_file(curr_xyz_name, *final_xyz_name)) + die(_("unable to rename temporary '*.%s' file to '%s"), + ext, *final_xyz_name); + } else if (else_chmod_if) { + chmod(*final_xyz_name, 0444); + } +}
"else_chmod_if" is unclear. The caller must be aware of what 'else' refers to and anybody adding a new caller is forced to go look at the body of this helper. I think chmod (or "make_read_only") happens only when the current one already has the final name, so perhaps that is what the name should highlight? make-read-only-if-same or somesuch? Thanks.