On Mon, Apr 07, 2025 at 05:35:47PM -0600, Gustavo A. R. Silva wrote:
> [..]
>
> > > > > - struct {
> > > > > - struct nvif_chan_v0 chan;
> > > > > - char name[TASK_COMM_LEN+16];
> > > > > - } args;
> > > > > + DEFINE_RAW_FLEX(struct nvif_chan_v0, args, name, TASK_COMM_LEN
> > > > > + 16);
> > > > > struct nvif_device *device = &cli->device;
> > > > > struct nouveau_channel *chan;
> > > > > const u64 plength = 0x10000;
> > > > > @@ -298,28 +295,28 @@ nouveau_channel_ctor(struct nouveau_cli *cli,
> > > > > bool priv, u64 runm,
> > > > > return ret;
> > > > > /* create channel object */
> > > > > - args.chan.version = 0;
> > > > > - args.chan.namelen = sizeof(args.name);
> > > > > - args.chan.runlist = __ffs64(runm);
> > > > > - args.chan.runq = 0;
> > > > > - args.chan.priv = priv;
> > > > > - args.chan.devm = BIT(0);
> > > > > + args->version = 0;
> > > > > + args->namelen = __struct_size(args) - sizeof(*args);
> > > >
> > > > Does __struct_size(args->name) work here (and later)?
> > >
> > > Why not?
> >
> > Uhm, I'm genuinely curious. I *think* it will work, but because it's
> > within the struct, not outside of it, I'm unclear if it'll DTRT for
> > finding the size (since __builtin_object_size() can be touchy).
> >
> > > I mean, this should be equivalent to `TASK_COMM_LEN+16`, I could
> > > use the latter if people prefer it (see my comments below).
> >
> > Right, it should be the same. I think __struct_size(args->name) would be
> > much more readable ... if it works. :)
>
> OK, I'll double check this.
Ah-ha, yes, I'm already testing this with KUnit:
struct bar {
int a;
u32 counter;
s16 array[];
};
...
DEFINE_RAW_FLEX(struct bar, two, array, 2);
...
KUNIT_EXPECT_EQ(test, sizeof(*two), sizeof(struct bar));
KUNIT_EXPECT_EQ(test, __struct_size(two), sizeof(struct bar) + 2 *
sizeof(s16));
KUNIT_EXPECT_EQ(test, __member_size(two), sizeof(struct bar) + 2 *
sizeof(s16));
KUNIT_EXPECT_EQ(test, __struct_size(two->array), 2 * sizeof(s16));
KUNIT_EXPECT_EQ(test, __member_size(two->array), 2 * sizeof(s16));
> I really don't want to condition -Wflex-array-member-not-at-end patches
> on counted_by patches, for now.
Fair enough. :) One thing at a time is wise!
--
Kees Cook
