Re: [PATCH v2 01/18] cleanup: rename variables that collide with Rust primitive type names

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 17, 2025, 14:32 UTC
Message-ID: <xmqqa52tgne2.fsf@gitster.g>
In-Reply-To: <CAPig+cQqKCbGpfp=ppmjKEOe+sDRu6BocDfenzqvQJHSMiKDHQ@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 8 quoted lines

> Same comment applies to the other renamed variables...
>
>> -               OPT_UNSIGNED(0, "u16", &u16, "get a 16 bit unsigned integer"),
>> +               OPT_UNSIGNED(0, "u16", &u16_, "get a 16 bit unsigned integer"),
>
> ... though with some of them, such as this one, it is admittedly more
> difficult to come up with a better name since the original name is
> already meaningful.

On top of that, just like a name that begins with an underscore is reserved by C standard, a name taht ends with an underscore is used for specific purposes by convention in this project.

Typically 'foo_' appears as a parameter to a function, whose type is specified with our interhal API, and when that type is cumbersome to work with, we have a local equivalent 'foo' of a more appropriate type whose value is populated from 'foo_' before being used, and use of 'foo' thanks to its better type is more ergonomic.

    static int compare_pt(const void *a_, const void *b_)
    {
            struct possible_tag *a = (struct possible_tag *)a_;
            struct possible_tag *b = (struct possible_tag *)b_;
            if (a->depth != b->depth)
                    return a->depth - b->depth;
            if (a->found_order != b->found_order)
                    return a->found_order - b->found_order;
            return 0;
    }

    static int path_is_beyond_symlink(struct apply_state *state, const char *name_)
    {
            int ret;
            struct strbuf name = STRBUF_INIT;

            assert(*name_ != '\0');
            strbuf_addstr(&name, name_);
            ret = path_is_beyond_symlink_1(state, &name);
            strbuf_release(&name);

            return ret;
    }

are examples.

There are existing crappy code that uses foo_ without corresponding foo; we should clean them up, not emulating or spreading the pattern.

By the way, in this partcular case, why not use "uint16_t u16"?

Isn't the true cause of the trouble the (I might say "misguided") desire to use "u16" as a type in C code? As long as we all agree that the data that can be passed across the ffi barrier should be of the types of known size, and let C side use uint(8|16|32|64)_t and Rust side use u(8|16|32|64) consistently, we do not need to have this "cleanup", do we?

Thanks.