Re: [PATCH][next] ALSA: firewire-lib: Fix uninitialized variable err issue

[Takashi Sakamoto]

Hi,

On Thu, May 20, 2021 at 02:26:24PM +0200, Takashi Iwai wrote:
> On Thu, 20 May 2021 10:34:24 +0200,
> Colin King wrote:
> > 
> > From: Colin Ian King <colin.king@xxxxxxxxxxxxx>
> > 
> > Currently in the case where the payload_length is less than the
> > cip_header_size the error return variable err is not being set
> > and function parse_ir_ctx_header can return an uninitialized
> > error return value. Fix this by setting err to zero.
> > 
> > Addresses-Coverity: ("Uninitialized scalar variable")
> > Fixes: c09010eeb373 ("ALSA: firewire-lib: handle the case that empty isochronous packet payload for CIP")
> > Signed-off-by: Colin Ian King <colin.king@xxxxxxxxxxxxx>
> 
> Just a bikeshed, IMO, it'd be more proper to initialize err at the
> beginning than setting 0 at every branch, e.g.
> 
> --- a/sound/firewire/amdtp-stream.c
> +++ b/sound/firewire/amdtp-stream.c
> @@ -652,7 +652,7 @@ static int parse_ir_ctx_header(struct amdtp_stream *s, unsigned int cycle,
>  	unsigned int payload_length;
>  	const __be32 *cip_header;
>  	unsigned int cip_header_size;
> -	int err;
> +	int err = 0;
>  
>  	payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT;
>  
> @@ -683,7 +683,6 @@ static int parse_ir_ctx_header(struct amdtp_stream *s, unsigned int cycle,
>  		}
>  	} else {
>  		cip_header = NULL;
> -		err = 0;
>  		*data_blocks = payload_length / sizeof(__be32) / s->data_block_quadlets;
>  		*syt = 0;

Thanks for the patches.

The error check is just done for the case to process CIP header, thus we
can put the auto variable into the branch.

======== 8< --------