Re: [PATCH 6/9] index-pack: refactor renaming in final()

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.

+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.

On Thu, Sep 09, 2021 at 12:45:04PM -0700, Junio C Hamano wrote:

Taylor Blau [off-list ref] writes:

quoted

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.

[...]

make-read-only-if-same or somesuch?

Both of these suggestions (dropping the "xyz"s and renaming the last
parameter to "make_read_only_if_same") make sense. Will apply.

Thanks,
Taylor