Hi Srinivas,
First of all, thanks for looking at this ;)

On Sat, Jun 08, 2024 at 01:42:54AM -0700, srinivas pandruvada wrote:

On Sun, 2024-05-26 at 15:32 +0200, Erick Archer wrote:

quoted

One-element arrays as fake flex arrays are deprecated [1] and we are
moving towards adopting C99 flexible-array members, instead. This
case
also has more complexity because it is a flexible array of flexible
arrays and this patch needs to be ready to enable the new compiler
flag
-Wflex-array-member-not-at-end (coming in GCC-14) globally.

So, define a new struct type for the single reports:

struct report {
	uint16_t size;
	struct hostif_msg_hdr msg;
} __packed;

but without the payload (flex array) in it. And add this payload to
the
"hostif_msg" structure. This way, in the "report_list" structure we
can
declare a flex array of single reports which now do not contain
another
flex array.

struct report_list {
	[...]
        struct report reports[];
} __packed;

Also, use "container_of()" whenever we need to retrieve a pointer to
the flexible structure, through which we can access the flexible
array
if needed.

Moreover, refactor the code accordingly to use the new structures and
take advantage of this avoiding some pointer arithmetic and using the
"struct_size" helper when possible.

This way, the code is more readable and safer.

Applied and tested, atleast didn't break anything.

But the explanation above didn't give me enough clue. You have added a
payload[] in the  struct hostif_msg {} then using that as a message
pointer following the header. I think this description needs to be
better.

Yeah, I will try to improve the commit message. What do you think about
the following parragrafs?

[I have copied part of the message to show where the new info will be]

quoted

declare a flex array of single reports which now do not contain
another flex array.

struct report_list {
	[...]
        struct report reports[];
} __packed;

Therefore, the "struct hostif_msg" is now made up of a header and a
payload. And the "struct report" uses only the "hostif_msg" header.
The perfect solution would be for the "report" structure to use the
whole "hostif_msg" structure but this is not possible due to nested
flexible arrays. Anyway, the end result is equivalent since this patch
does attemp to change the behaviour of the code.

Now as well, we have more clarity after the cast from the raw bytes to
the new structures.

quoted

Also, use "container_of()" whenever we need to retrieve a pointer to
the flexible structure, through which we can access the flexible
array
if needed.

I would like to know if it is enough :)

Regards,
Erick

Thanks,
Srinivas