All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] ccs: Make (non-)use of uninitialised variables more robust
@ 2021-01-05 12:49 Sakari Ailus
  2021-01-05 16:18 ` Hans Verkuil
  0 siblings, 1 reply; 3+ messages in thread
From: Sakari Ailus @ 2021-01-05 12:49 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil

GCC with W=2 level of kernel compiler warnings warns about the use of
uninitialised variables in a few locations. While these uninitialised
variables were not used in reality, this still produced compiler warnings.

Address this by assigning the variables to NULL and checking for NULL in
places it is not expected, returning -EPROTO in that case. This provides
at least some sanity checking at runtime as the compiler appears unable to
do that at compile time.

Reported-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-data.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-data.c b/drivers/media/i2c/ccs/ccs-data.c
index 59338a6704af..99b2b515058a 100644
--- a/drivers/media/i2c/ccs/ccs-data.c
+++ b/drivers/media/i2c/ccs/ccs-data.c
@@ -214,7 +214,7 @@ static int ccs_data_parse_regs(struct bin_container *bin,
 			       size_t *__num_regs, const void *payload,
 			       const void *endp, struct device *dev)
 {
-	struct ccs_reg *regs_base, *regs;
+	struct ccs_reg *regs_base = NULL, *regs = NULL;
 	size_t num_regs = 0;
 	u16 addr = 0;
 
@@ -285,6 +285,9 @@ static int ccs_data_parse_regs(struct bin_container *bin,
 		if (!bin->base) {
 			bin_reserve(bin, len);
 		} else if (__regs) {
+			if (!regs)
+				return -EPROTO;
+
 			regs->addr = addr;
 			regs->len = len;
 			regs->value = bin_alloc(bin, len);
@@ -305,8 +308,12 @@ static int ccs_data_parse_regs(struct bin_container *bin,
 	if (__num_regs)
 		*__num_regs = num_regs;
 
-	if (bin->base && __regs)
+	if (bin->base && __regs) {
+		if (!regs_base)
+			return -EPROTO;
+
 		*__regs = regs_base;
+	}
 
 	return 0;
 }
@@ -425,7 +432,7 @@ static int ccs_data_parse_rules(struct bin_container *bin,
 				size_t *__num_rules, const void *payload,
 				const void *endp, struct device *dev)
 {
-	struct ccs_rule *rules_base, *rules = NULL, *next_rule;
+	struct ccs_rule *rules_base = NULL, *rules = NULL, *next_rule = NULL;
 	size_t num_rules = 0;
 	const void *__next_rule = payload;
 	int rval;
@@ -483,6 +490,9 @@ static int ccs_data_parse_rules(struct bin_container *bin,
 			} else {
 				unsigned int i;
 
+				if (!next_rule)
+					return -EPROTO;
+
 				rules = next_rule;
 				next_rule++;
 
@@ -555,6 +565,9 @@ static int ccs_data_parse_rules(struct bin_container *bin,
 		bin_reserve(bin, sizeof(*rules) * num_rules);
 		*__num_rules = num_rules;
 	} else {
+		if (!rules_base)
+			return -EPROTO;
+
 		*__rules = rules_base;
 	}
 
@@ -690,7 +703,7 @@ static int ccs_data_parse_pdaf(struct bin_container *bin, struct ccs_pdaf_pix_lo
 	}
 
 	for (i = 0; i < max_block_type_id; i++) {
-		struct ccs_pdaf_pix_loc_pixel_desc_group *pdgroup;
+		struct ccs_pdaf_pix_loc_pixel_desc_group *pdgroup = NULL;
 		unsigned int j;
 
 		if (!is_contained(__num_pixel_descs, endp))
@@ -721,6 +734,9 @@ static int ccs_data_parse_pdaf(struct bin_container *bin, struct ccs_pdaf_pix_lo
 			if (!bin->base)
 				continue;
 
+			if (!pdgroup)
+				return -EPROTO;
+
 			pdesc = &pdgroup->descs[j];
 			pdesc->pixel_type = __pixel_desc->pixel_type;
 			pdesc->small_offset_x = __pixel_desc->small_offset_x;
-- 
2.29.2


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

* Re: [PATCH 1/1] ccs: Make (non-)use of uninitialised variables more robust
  2021-01-05 12:49 [PATCH 1/1] ccs: Make (non-)use of uninitialised variables more robust Sakari Ailus
@ 2021-01-05 16:18 ` Hans Verkuil
  2021-01-05 17:17   ` Sakari Ailus
  0 siblings, 1 reply; 3+ messages in thread
From: Hans Verkuil @ 2021-01-05 16:18 UTC (permalink / raw)
  To: Sakari Ailus, linux-media

On 05/01/2021 13:49, Sakari Ailus wrote:
> GCC with W=2 level of kernel compiler warnings warns about the use of
> uninitialised variables in a few locations. While these uninitialised
> variables were not used in reality, this still produced compiler warnings.
> 
> Address this by assigning the variables to NULL and checking for NULL in
> places it is not expected, returning -EPROTO in that case. This provides
> at least some sanity checking at runtime as the compiler appears unable to
> do that at compile time.
> 
> Reported-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

This fixes the warnings!

Thank you,

	Hans

> ---
>  drivers/media/i2c/ccs/ccs-data.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/ccs/ccs-data.c b/drivers/media/i2c/ccs/ccs-data.c
> index 59338a6704af..99b2b515058a 100644
> --- a/drivers/media/i2c/ccs/ccs-data.c
> +++ b/drivers/media/i2c/ccs/ccs-data.c
> @@ -214,7 +214,7 @@ static int ccs_data_parse_regs(struct bin_container *bin,
>  			       size_t *__num_regs, const void *payload,
>  			       const void *endp, struct device *dev)
>  {
> -	struct ccs_reg *regs_base, *regs;
> +	struct ccs_reg *regs_base = NULL, *regs = NULL;
>  	size_t num_regs = 0;
>  	u16 addr = 0;
>  
> @@ -285,6 +285,9 @@ static int ccs_data_parse_regs(struct bin_container *bin,
>  		if (!bin->base) {
>  			bin_reserve(bin, len);
>  		} else if (__regs) {
> +			if (!regs)
> +				return -EPROTO;
> +
>  			regs->addr = addr;
>  			regs->len = len;
>  			regs->value = bin_alloc(bin, len);
> @@ -305,8 +308,12 @@ static int ccs_data_parse_regs(struct bin_container *bin,
>  	if (__num_regs)
>  		*__num_regs = num_regs;
>  
> -	if (bin->base && __regs)
> +	if (bin->base && __regs) {
> +		if (!regs_base)
> +			return -EPROTO;
> +
>  		*__regs = regs_base;
> +	}
>  
>  	return 0;
>  }
> @@ -425,7 +432,7 @@ static int ccs_data_parse_rules(struct bin_container *bin,
>  				size_t *__num_rules, const void *payload,
>  				const void *endp, struct device *dev)
>  {
> -	struct ccs_rule *rules_base, *rules = NULL, *next_rule;
> +	struct ccs_rule *rules_base = NULL, *rules = NULL, *next_rule = NULL;
>  	size_t num_rules = 0;
>  	const void *__next_rule = payload;
>  	int rval;
> @@ -483,6 +490,9 @@ static int ccs_data_parse_rules(struct bin_container *bin,
>  			} else {
>  				unsigned int i;
>  
> +				if (!next_rule)
> +					return -EPROTO;
> +
>  				rules = next_rule;
>  				next_rule++;
>  
> @@ -555,6 +565,9 @@ static int ccs_data_parse_rules(struct bin_container *bin,
>  		bin_reserve(bin, sizeof(*rules) * num_rules);
>  		*__num_rules = num_rules;
>  	} else {
> +		if (!rules_base)
> +			return -EPROTO;
> +
>  		*__rules = rules_base;
>  	}
>  
> @@ -690,7 +703,7 @@ static int ccs_data_parse_pdaf(struct bin_container *bin, struct ccs_pdaf_pix_lo
>  	}
>  
>  	for (i = 0; i < max_block_type_id; i++) {
> -		struct ccs_pdaf_pix_loc_pixel_desc_group *pdgroup;
> +		struct ccs_pdaf_pix_loc_pixel_desc_group *pdgroup = NULL;
>  		unsigned int j;
>  
>  		if (!is_contained(__num_pixel_descs, endp))
> @@ -721,6 +734,9 @@ static int ccs_data_parse_pdaf(struct bin_container *bin, struct ccs_pdaf_pix_lo
>  			if (!bin->base)
>  				continue;
>  
> +			if (!pdgroup)
> +				return -EPROTO;
> +
>  			pdesc = &pdgroup->descs[j];
>  			pdesc->pixel_type = __pixel_desc->pixel_type;
>  			pdesc->small_offset_x = __pixel_desc->small_offset_x;
> 


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

* Re: [PATCH 1/1] ccs: Make (non-)use of uninitialised variables more robust
  2021-01-05 16:18 ` Hans Verkuil
@ 2021-01-05 17:17   ` Sakari Ailus
  0 siblings, 0 replies; 3+ messages in thread
From: Sakari Ailus @ 2021-01-05 17:17 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media

On Tue, Jan 05, 2021 at 05:18:08PM +0100, Hans Verkuil wrote:
> On 05/01/2021 13:49, Sakari Ailus wrote:
> > GCC with W=2 level of kernel compiler warnings warns about the use of
> > uninitialised variables in a few locations. While these uninitialised
> > variables were not used in reality, this still produced compiler warnings.
> > 
> > Address this by assigning the variables to NULL and checking for NULL in
> > places it is not expected, returning -EPROTO in that case. This provides
> > at least some sanity checking at runtime as the compiler appears unable to
> > do that at compile time.
> > 
> > Reported-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> This fixes the warnings!

Thanks!

I think I'll switch the error code to something else. -EPROTO is already in
use, so I'll send v2 with -EIO.

-- 
Sakari Ailus

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

end of thread, other threads:[~2021-01-05 17:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-05 12:49 [PATCH 1/1] ccs: Make (non-)use of uninitialised variables more robust Sakari Ailus
2021-01-05 16:18 ` Hans Verkuil
2021-01-05 17:17   ` Sakari Ailus

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.