All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: venus: Replace zero-length array with flexible-array
@ 2020-05-07 19:07 Gustavo A. R. Silva
  2020-05-07 20:39 ` Stanimir Varbanov
  0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2020-05-07 19:07 UTC (permalink / raw)
  To: Stanimir Varbanov; +Cc: linux-media, linux-arm-msm, linux-kernel

The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:

struct foo {
        int stuff;
        struct boo array[];
};

By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.

Also, notice that, dynamic memory allocations won't be affected by
this change:

"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]

sizeof(flexible-array-member) triggers a warning because flexible array
members have incomplete type[1]. There are some instances of code in
which the sizeof operator is being incorrectly/erroneously applied to
zero-length arrays and the result is zero. Such instances may be hiding
some bugs. So, this work (flexible-array member conversions) will also
help to get completely rid of those sorts of issues.

This issue was found with the help of Coccinelle.

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 drivers/media/platform/qcom/venus/hfi_cmds.h |    2 +-
 drivers/media/platform/qcom/venus/hfi_msgs.h |   10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.h b/drivers/media/platform/qcom/venus/hfi_cmds.h
index cae9d5d61c0c..83705e237f1c 100644
--- a/drivers/media/platform/qcom/venus/hfi_cmds.h
+++ b/drivers/media/platform/qcom/venus/hfi_cmds.h
@@ -107,7 +107,7 @@ struct hfi_session_abort_pkt {
 struct hfi_session_set_property_pkt {
 	struct hfi_session_hdr_pkt shdr;
 	u32 num_properties;
-	u32 data[0];
+	u32 data[];
 };
 
 struct hfi_session_set_buffers_pkt {
diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.h b/drivers/media/platform/qcom/venus/hfi_msgs.h
index 7694b1d25d9d..526d9f5b487b 100644
--- a/drivers/media/platform/qcom/venus/hfi_msgs.h
+++ b/drivers/media/platform/qcom/venus/hfi_msgs.h
@@ -155,7 +155,7 @@ struct hfi_msg_session_empty_buffer_done_pkt {
 	u32 input_tag;
 	u32 packet_buffer;
 	u32 extradata_buffer;
-	u32 data[0];
+	u32 data[];
 };
 
 struct hfi_msg_session_fbd_compressed_pkt {
@@ -175,7 +175,7 @@ struct hfi_msg_session_fbd_compressed_pkt {
 	u32 picture_type;
 	u32 packet_buffer;
 	u32 extradata_buffer;
-	u32 data[0];
+	u32 data[];
 };
 
 struct hfi_msg_session_fbd_uncompressed_plane0_pkt {
@@ -202,7 +202,7 @@ struct hfi_msg_session_fbd_uncompressed_plane0_pkt {
 	u32 picture_type;
 	u32 packet_buffer;
 	u32 extradata_buffer;
-	u32 data[0];
+	u32 data[];
 };
 
 struct hfi_msg_session_fbd_uncompressed_plane1_pkt {
@@ -211,7 +211,7 @@ struct hfi_msg_session_fbd_uncompressed_plane1_pkt {
 	u32 filled_len;
 	u32 offset;
 	u32 packet_buffer2;
-	u32 data[0];
+	u32 data[];
 };
 
 struct hfi_msg_session_fbd_uncompressed_plane2_pkt {
@@ -220,7 +220,7 @@ struct hfi_msg_session_fbd_uncompressed_plane2_pkt {
 	u32 filled_len;
 	u32 offset;
 	u32 packet_buffer3;
-	u32 data[0];
+	u32 data[];
 };
 
 struct hfi_msg_session_parse_sequence_header_done_pkt {


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: venus: Replace zero-length array with flexible-array
  2020-05-07 19:07 [PATCH] media: venus: Replace zero-length array with flexible-array Gustavo A. R. Silva
@ 2020-05-07 20:39 ` Stanimir Varbanov
  2020-05-07 21:18   ` Gustavo A. R. Silva
  0 siblings, 1 reply; 3+ messages in thread
From: Stanimir Varbanov @ 2020-05-07 20:39 UTC (permalink / raw)
  To: Gustavo A. R. Silva; +Cc: linux-media, linux-arm-msm, linux-kernel

Hi Gustavo,

Two patches from you are already queued in media_tree for v5.8.

0f61e171e4bbac4595175070c75707f1b12f4e37 media: venus: hfi_msgs.h:
Replace zero-length array with flexible-array member

380f3bbd9562dc93be2e3cadc329b15284fbedae media: venus: hfi_cmds.h:
Replace zero-length array with flexible-array member


On 5/7/20 10:07 PM, Gustavo A. R. Silva wrote:
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
> 
> struct foo {
>         int stuff;
>         struct boo array[];
> };
> 
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
> 
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
> 
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
> 
> sizeof(flexible-array-member) triggers a warning because flexible array
> members have incomplete type[1]. There are some instances of code in
> which the sizeof operator is being incorrectly/erroneously applied to
> zero-length arrays and the result is zero. Such instances may be hiding
> some bugs. So, this work (flexible-array member conversions) will also
> help to get completely rid of those sorts of issues.
> 
> This issue was found with the help of Coccinelle.
> 
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  drivers/media/platform/qcom/venus/hfi_cmds.h |    2 +-
>  drivers/media/platform/qcom/venus/hfi_msgs.h |   10 +++++-----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.h b/drivers/media/platform/qcom/venus/hfi_cmds.h
> index cae9d5d61c0c..83705e237f1c 100644
> --- a/drivers/media/platform/qcom/venus/hfi_cmds.h
> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.h
> @@ -107,7 +107,7 @@ struct hfi_session_abort_pkt {
>  struct hfi_session_set_property_pkt {
>  	struct hfi_session_hdr_pkt shdr;
>  	u32 num_properties;
> -	u32 data[0];
> +	u32 data[];
>  };
>  
>  struct hfi_session_set_buffers_pkt {
> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.h b/drivers/media/platform/qcom/venus/hfi_msgs.h
> index 7694b1d25d9d..526d9f5b487b 100644
> --- a/drivers/media/platform/qcom/venus/hfi_msgs.h
> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.h
> @@ -155,7 +155,7 @@ struct hfi_msg_session_empty_buffer_done_pkt {
>  	u32 input_tag;
>  	u32 packet_buffer;
>  	u32 extradata_buffer;
> -	u32 data[0];
> +	u32 data[];
>  };
>  
>  struct hfi_msg_session_fbd_compressed_pkt {
> @@ -175,7 +175,7 @@ struct hfi_msg_session_fbd_compressed_pkt {
>  	u32 picture_type;
>  	u32 packet_buffer;
>  	u32 extradata_buffer;
> -	u32 data[0];
> +	u32 data[];
>  };
>  
>  struct hfi_msg_session_fbd_uncompressed_plane0_pkt {
> @@ -202,7 +202,7 @@ struct hfi_msg_session_fbd_uncompressed_plane0_pkt {
>  	u32 picture_type;
>  	u32 packet_buffer;
>  	u32 extradata_buffer;
> -	u32 data[0];
> +	u32 data[];
>  };
>  
>  struct hfi_msg_session_fbd_uncompressed_plane1_pkt {
> @@ -211,7 +211,7 @@ struct hfi_msg_session_fbd_uncompressed_plane1_pkt {
>  	u32 filled_len;
>  	u32 offset;
>  	u32 packet_buffer2;
> -	u32 data[0];
> +	u32 data[];
>  };
>  
>  struct hfi_msg_session_fbd_uncompressed_plane2_pkt {
> @@ -220,7 +220,7 @@ struct hfi_msg_session_fbd_uncompressed_plane2_pkt {
>  	u32 filled_len;
>  	u32 offset;
>  	u32 packet_buffer3;
> -	u32 data[0];
> +	u32 data[];
>  };
>  
>  struct hfi_msg_session_parse_sequence_header_done_pkt {
> 

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: venus: Replace zero-length array with flexible-array
  2020-05-07 20:39 ` Stanimir Varbanov
@ 2020-05-07 21:18   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2020-05-07 21:18 UTC (permalink / raw)
  To: Stanimir Varbanov; +Cc: linux-media, linux-arm-msm, linux-kernel

On Thu, May 07, 2020 at 11:39:51PM +0300, Stanimir Varbanov wrote:
> Hi Gustavo,
> 
> Two patches from you are already queued in media_tree for v5.8.
> 

Awesome. :)

Thanks, Stanimir.
--
Gustavo

> 0f61e171e4bbac4595175070c75707f1b12f4e37 media: venus: hfi_msgs.h:
> Replace zero-length array with flexible-array member
> 
> 380f3bbd9562dc93be2e3cadc329b15284fbedae media: venus: hfi_cmds.h:
> Replace zero-length array with flexible-array member
> 
> 
> On 5/7/20 10:07 PM, Gustavo A. R. Silva wrote:
> > The current codebase makes use of the zero-length array language
> > extension to the C90 standard, but the preferred mechanism to declare
> > variable-length types such as these ones is a flexible array member[1][2],
> > introduced in C99:
> > 
> > struct foo {
> >         int stuff;
> >         struct boo array[];
> > };
> > 
> > By making use of the mechanism above, we will get a compiler warning
> > in case the flexible array does not occur last in the structure, which
> > will help us prevent some kind of undefined behavior bugs from being
> > inadvertently introduced[3] to the codebase from now on.
> > 
> > Also, notice that, dynamic memory allocations won't be affected by
> > this change:
> > 
> > "Flexible array members have incomplete type, and so the sizeof operator
> > may not be applied. As a quirk of the original implementation of
> > zero-length arrays, sizeof evaluates to zero."[1]
> > 
> > sizeof(flexible-array-member) triggers a warning because flexible array
> > members have incomplete type[1]. There are some instances of code in
> > which the sizeof operator is being incorrectly/erroneously applied to
> > zero-length arrays and the result is zero. Such instances may be hiding
> > some bugs. So, this work (flexible-array member conversions) will also
> > help to get completely rid of those sorts of issues.
> > 
> > This issue was found with the help of Coccinelle.
> > 
> > [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> > [2] https://github.com/KSPP/linux/issues/21
> > [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
> > 
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > ---
> >  drivers/media/platform/qcom/venus/hfi_cmds.h |    2 +-
> >  drivers/media/platform/qcom/venus/hfi_msgs.h |   10 +++++-----
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.h b/drivers/media/platform/qcom/venus/hfi_cmds.h
> > index cae9d5d61c0c..83705e237f1c 100644
> > --- a/drivers/media/platform/qcom/venus/hfi_cmds.h
> > +++ b/drivers/media/platform/qcom/venus/hfi_cmds.h
> > @@ -107,7 +107,7 @@ struct hfi_session_abort_pkt {
> >  struct hfi_session_set_property_pkt {
> >  	struct hfi_session_hdr_pkt shdr;
> >  	u32 num_properties;
> > -	u32 data[0];
> > +	u32 data[];
> >  };
> >  
> >  struct hfi_session_set_buffers_pkt {
> > diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.h b/drivers/media/platform/qcom/venus/hfi_msgs.h
> > index 7694b1d25d9d..526d9f5b487b 100644
> > --- a/drivers/media/platform/qcom/venus/hfi_msgs.h
> > +++ b/drivers/media/platform/qcom/venus/hfi_msgs.h
> > @@ -155,7 +155,7 @@ struct hfi_msg_session_empty_buffer_done_pkt {
> >  	u32 input_tag;
> >  	u32 packet_buffer;
> >  	u32 extradata_buffer;
> > -	u32 data[0];
> > +	u32 data[];
> >  };
> >  
> >  struct hfi_msg_session_fbd_compressed_pkt {
> > @@ -175,7 +175,7 @@ struct hfi_msg_session_fbd_compressed_pkt {
> >  	u32 picture_type;
> >  	u32 packet_buffer;
> >  	u32 extradata_buffer;
> > -	u32 data[0];
> > +	u32 data[];
> >  };
> >  
> >  struct hfi_msg_session_fbd_uncompressed_plane0_pkt {
> > @@ -202,7 +202,7 @@ struct hfi_msg_session_fbd_uncompressed_plane0_pkt {
> >  	u32 picture_type;
> >  	u32 packet_buffer;
> >  	u32 extradata_buffer;
> > -	u32 data[0];
> > +	u32 data[];
> >  };
> >  
> >  struct hfi_msg_session_fbd_uncompressed_plane1_pkt {
> > @@ -211,7 +211,7 @@ struct hfi_msg_session_fbd_uncompressed_plane1_pkt {
> >  	u32 filled_len;
> >  	u32 offset;
> >  	u32 packet_buffer2;
> > -	u32 data[0];
> > +	u32 data[];
> >  };
> >  
> >  struct hfi_msg_session_fbd_uncompressed_plane2_pkt {
> > @@ -220,7 +220,7 @@ struct hfi_msg_session_fbd_uncompressed_plane2_pkt {
> >  	u32 filled_len;
> >  	u32 offset;
> >  	u32 packet_buffer3;
> > -	u32 data[0];
> > +	u32 data[];
> >  };
> >  
> >  struct hfi_msg_session_parse_sequence_header_done_pkt {
> > 
> 
> -- 
> regards,
> Stan

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-05-07 21:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 19:07 [PATCH] media: venus: Replace zero-length array with flexible-array Gustavo A. R. Silva
2020-05-07 20:39 ` Stanimir Varbanov
2020-05-07 21:18   ` Gustavo A. R. Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.