Taylor Blau <me@xxxxxxxxxxxx> 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.
