diff mbox series

[v2,18/18] media: venus: Refactor hfi_buffer_alloc_mode_supported

Message ID 20240507-cocci-flexarray-v2-18-7aea262cf065@chromium.org (mailing list archive)
State Superseded
Headers show
Series media: Fix the last set of coccinelle warnings | expand

Commit Message

Ricardo Ribalda May 7, 2024, 4:27 p.m. UTC
Replace the old style single element array at the end of the struct with
a flex array.

The code does not allocate this structure, so the size change should not
be a problem.

This fixes the following cocci warning:
drivers/media/platform/qcom/venus/hfi_helper.h:1233:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/platform/qcom/venus/hfi_helper.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bryan O'Donoghue May 10, 2024, 12:09 a.m. UTC | #1
On 07/05/2024 17:27, Ricardo Ribalda wrote:
> Replace the old style single element array at the end of the struct with
> a flex array.
> 
> The code does not allocate this structure, so the size change should not
> be a problem.
> 
> This fixes the following cocci warning:
> drivers/media/platform/qcom/venus/hfi_helper.h:1233:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>   drivers/media/platform/qcom/venus/hfi_helper.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
> index dee439ea4d2e..9545c964a428 100644
> --- a/drivers/media/platform/qcom/venus/hfi_helper.h
> +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
> @@ -1230,7 +1230,7 @@ struct hfi_interlace_format_supported {
>   struct hfi_buffer_alloc_mode_supported {
>   	u32 buffer_type;
>   	u32 num_entries;
> -	u32 data[1];
> +	u32 data[];
>   };
>   
>   struct hfi_metadata_pass_through {
> 

You have some fairly inconsistent fixes for this class.

Please don't change the sizeof() any structures in your series, because 
the structure is unallocated changing the size is potentially insidious IMO.

data[1] -> data is perfectly fine in this case.

---
bod
Ricardo Ribalda May 22, 2024, 1:03 p.m. UTC | #2
Hi Bryan

On Fri, 10 May 2024 at 02:09, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 07/05/2024 17:27, Ricardo Ribalda wrote:
> > Replace the old style single element array at the end of the struct with
> > a flex array.
> >
> > The code does not allocate this structure, so the size change should not
> > be a problem.
> >
> > This fixes the following cocci warning:
> > drivers/media/platform/qcom/venus/hfi_helper.h:1233:5-9: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >   drivers/media/platform/qcom/venus/hfi_helper.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
> > index dee439ea4d2e..9545c964a428 100644
> > --- a/drivers/media/platform/qcom/venus/hfi_helper.h
> > +++ b/drivers/media/platform/qcom/venus/hfi_helper.h
> > @@ -1230,7 +1230,7 @@ struct hfi_interlace_format_supported {
> >   struct hfi_buffer_alloc_mode_supported {
> >       u32 buffer_type;
> >       u32 num_entries;
> > -     u32 data[1];
> > +     u32 data[];
> >   };
> >
> >   struct hfi_metadata_pass_through {
> >
>
> You have some fairly inconsistent fixes for this class.
>
> Please don't change the sizeof() any structures in your series, because
> the structure is unallocated changing the size is potentially insidious IMO.

If the array is a flex array we should convert it to a real flex
array. Abusing one element arrays is deprecated.

If the driver only accesses the first element of the array, I have
modified the code from data[1] to data, because, as you say, modifying
the struct size can have dangerous side effects.

But if the driver accesses more fields, then I have reviewed that
there are no allocations, or any calculations based on the structure
side, and convert them to a proper flex array.


Regards!
>
> data[1] -> data is perfectly fine in this case.

If you take a look at parse_alloc_mode() you will see that mode->data
is indeed an array, and it is used by the pointer type.

>
> ---
> bod
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
index dee439ea4d2e..9545c964a428 100644
--- a/drivers/media/platform/qcom/venus/hfi_helper.h
+++ b/drivers/media/platform/qcom/venus/hfi_helper.h
@@ -1230,7 +1230,7 @@  struct hfi_interlace_format_supported {
 struct hfi_buffer_alloc_mode_supported {
 	u32 buffer_type;
 	u32 num_entries;
-	u32 data[1];
+	u32 data[];
 };
 
 struct hfi_metadata_pass_through {