On Thu, Feb 18, 2021 at 11:58:33AM +0100, Ævar Arnfjörð Bjarmason wrote:
> Rename variables in a function added in 0282f4dced0 (fsck: offer a
> function to demote fsck errors to warnings, 2015-06-22).
>
> It was needlessly confusing that it took a "msg_type" argument, but
> then later declared another "msg_type" of a different type.
>
> Let's rename that to "tmp", and rename "id" to "msg_id" and "msg_id"
> to "msg_id_str" etc. This will make a follow-up change smaller.
I think this is an improvement, though maybe "severity" would be a
less-generic term than "type".
> void fsck_set_msg_type(struct fsck_options *options,
> - const char *msg_id, const char *msg_type)
> + const char *msg_id_str, const char *msg_type_str)
> {
> - int id = parse_msg_id(msg_id), type;
> + int msg_id = parse_msg_id(msg_id_str), msg_type;
I always get nervous when a refactoring renames something away from
"foo", and then renames another thing _to_ "foo". Any untouched bits of
code are vulnerable to confusing them.
But I think the types are sufficiently different that we can mostly rely
on the compiler (though things like numeric or bool comparisons can work
with either pointers or ints), and the fact that we can see the entire
function is small enough that we can see the entire thing in the context
here.
So I think it is OK.
-Peff
