Re: [PATCH][next] drm/nouveau: chan: Avoid -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

[..]

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

[..]

@@ -367,17 +365,17 @@ nouveau_channel_init(struct nouveau_channel *chan, u32 
vram, u32 gart)
                return ret;
        if (chan->user.oclass >= FERMI_CHANNEL_GPFIFO) {
-               struct {
-                       struct nvif_event_v0 base;
-                       struct nvif_chan_event_v0 host;
-               } args;
+               DEFINE_RAW_FLEX(struct nvif_event_v0, args, data,
+                               sizeof(struct nvif_chan_event_v0));
+               struct nvif_chan_event_v0 *host =
+                               (struct nvif_chan_event_v0 *)args->data;
-               args.host.version = 0;
-               args.host.type = NVIF_CHAN_EVENT_V0_KILLED;
+               host->version = 0;
+               host->type = NVIF_CHAN_EVENT_V0_KILLED;
                ret = nvif_event_ctor(&chan->user, "abi16ChanKilled", 
chan->chid,
                                      nouveau_channel_killed, false,
-                                     &args.base, sizeof(args), &chan->kill);
+                                     args, __struct_size(args), &chan->kill);
                if (ret == 0)
                        ret = nvif_event_allow(&chan->kill);
                if (ret) {
@@ -520,46 +518,45 @@ nouveau_channels_fini(struct nouveau_drm *drm)
   int
   nouveau_channels_init(struct nouveau_drm *drm)
   {
-       struct {
-               struct nv_device_info_v1 m;
-               struct {
-                       struct nv_device_info_v1_data channels;
-                       struct nv_device_info_v1_data runlists;
-               } v;
-       } args = {
-               .m.version = 1,
-               .m.count = sizeof(args.v) / sizeof(args.v.channels),

sizeof(args.v) == sizeof(struct nv_device_info_v1_data) * 2

and sizeof(args.v.channels) == sizeof(struct nv_device_info_v1_data).

Isn't this just "2"? i.e. isn't struct nv_device_info_v1::count the
counted_by for struct nv_device_info_v1::data?

Yes, it's just `2`. However, I didn't want to explicitly use the magic
number, in case people don't like it, as in other similar patches (in
other subsystems).

But, yeah, it's `2`. :)

Okay. So if "count" is set up as a counted_by, the assignment will
happen automatically (in DEFINE_FLEX -- no longer "RAW").

I really don't want to condition -Wflex-array-member-not-at-end patches
on counted_by patches, for now.

Thanks
--
Gustavo