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

From: Takashi Iwai <tiwai@suse.de>
To: Colin King <colin.king@canonical.com>
Cc: Clemens Ladisch <clemens@ladisch.de>,
	Takashi Sakamoto <o-takashi@sakamocchi.jp>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	alsa-devel@alsa-project.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH][next] ALSA: firewire-lib: Fix uninitialized variable err issue
Date: Thu, 20 May 2021 14:26:24 +0200	[thread overview]
Message-ID: <s5hh7ixh9qn.wl-tiwai@suse.de> (raw)
In-Reply-To: <20210520083424.6685-1-colin.king@canonical.com>

On Thu, 20 May 2021 10:34:24 +0200,
Colin King wrote:
> 
> From: Colin Ian King <colin.king@canonical.com>
> 
> 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@canonical.com>

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,

Takashi
