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

[Jeff King <peff@peff.net>]

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

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