Re: [PATCH 08/12] skip_prefix(): check const match between in and out params

[Phillip Wood <phillip.wood123@gmail.com>]

On 01/04/2026 20:24, Jeff King wrote:
> On Wed, Apr 01, 2026 at 03:04:10PM +0100, Phillip Wood wrote:
> 
>> On 01/04/2026 14:17, Phillip Wood wrote:
>>> On 01/04/2026 00:50, Jeff King wrote:
>>>>
>>>> +/*
>>>> + * Check that an out-parameter that is "at least as const as" a matching
>>>> + * in-parameter. For example, skip_prefix() will return "out" that
>>>> is a subset
>>>> + * of "str". So:
>>>> + *
>>>> + *  const str, const out: ok
>>>> + *  non-const str, const out: ok
>>>> + *  non-const str, non-const out: ok
>>>> + *  const str, non-const out: compile error
>>>> + *
>>>> + *  See the skip_prefix macro below for an example of use.
>>>> + */
>>>> +#define CONST_OUTPARAM(in, out) \
>>>> +    ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))
>>>> +#define skip_prefix(str, prefix, out) \
>>>> +    skip_prefix((str), (prefix), CONST_OUTPARAM((str), (out)))
>>>
>>> This is clever but it changes the behavior of skip_prefix() which is
>>> documented as not touching out if it returns false.
>>
>> Sorry, I've just realized we always take the other branch so this does not
>> change the behavior and is in fact a nice solution to the problem.
> 
> Yeah, exactly. I was curious if the dead branch would be left in place,
> but gcc seems to prune it even at -O0.
> 
> I also pondered whether:
> 
>    (*out = in,out)
> 
> might be a problem, but I think it is OK. The "," is a sequence point,
> so it is well defined (of course we would never run this code anyway,
> but if we have undefined behavior in the code at all, it may cause
> confusing effects).

I agree it's well defined and because this code is never executed we 
don't need to worry about evaluating "out" multiple times. While the 
message from using _Static_assert below is nicer I think having this 
which is simple and works with all compilers is a better trade off.

Thanks

Phillip

> For reference, this is the more complicated one I came up with:
> 
>    /*
>     * Note that builtin_types_compatible_p() counts "char" and "const
>     * char" as the same type. So we deref and construct our own pointer
>     * with const to find out it "x" is const, and then either compare
>     * x and y exactly (if it is const, they must both be) or dereferenced
>     * (which lets y be either const or not).
>     */
>    #define CONST_COMPATIBLE(x, y) \
>            (__builtin_types_compatible_p(typeof(x), const typeof(*(x)) *) ? \
>             __builtin_types_compatible_p(typeof(x), typeof(y)) : \
>             __builtin_types_compatible_p(typeof(*(x)), typeof(*(y))))
> 
> I also tried using a gcc statement-expression and _Static_assert to get
> a nicer message, like this:
> 
>    #define CONST_OUTPARAM(in, out, in_name, out_name) ({ \
>            _Static_assert(CONST_COMPATIBLE((in),*(out)), \
>                           in_name " is not const-compatible with " out_name); \
>            (const typeof(*(in)) **)(out); \
>    })
>    #define skip_prefix(str, prefix, out) \
>            skip_prefix(str, prefix, CONST_OUTPARAM((str), (out), #str, #out))
> 
> It does produce slightly nicer output:
> 
>    foo.c: In function ‘bad’:
>    foo.c:8:9: error: static assertion failed: "my_in_var is not const-compatible with my_out_var"
>        8 |         _Static_assert(CONST_COMPATIBLE((in),*(out)), \
>          |         ^~~~~~~~~~~~~~
>    foo.c:13:34: note: in expansion of macro ‘CONST_OUTPARAM’
>       13 |         skip_prefix(str, prefix, CONST_OUTPARAM((str), (out), #str, #out))
>          |                                  ^~~~~~~~~~~~~~
>    foo.c:16:9: note: in expansion of macro ‘skip_prefix’
>       16 |         skip_prefix(my_in_var, "foo", my_out_var);
> 
> but I don't think the extra complexity and portability headache is worth
> it.
> 
> -Peff