Re: Sparse errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 5/26/21 2:40 AM, Takashi Iwai wrote:
On Tue, 25 May 2021 21:32:27 +0200,
Pierre-Louis Bossart wrote:

Hi Takashi,
Sparse reports a lot of new issues in our last checks with more options:

export ARCH=x86_64 CF="-Wsparse-error -Wsparse-all
-Wno-bitwise-pointer -Wno-pointer-arith -Wno-typesign -Wnoshadow
-Wno-sizeof-bool"
make -k sound/ C=2

most are linked to the __user and pcm_format_t restricted types, but I
found the simpler ones below which are useless comparisons. I can send
a patch for the last but not sure how to address the first two.

Thanks for your feedback
-Pierre

sound/core/info.c:95:38: error: self-comparison always evaluates to false

	if (pos < 0 || (long) pos != pos || (ssize_t) count < 0)
		return false;

not sure what the second comparison is meant to check?

As Dan suggested, it's a check only for 32bit architecture for a 64bit
value.

Isn't there a better way to check this?

sound/drivers/opl3/opl3_midi.c:183:60: error: self-comparison always
evaluates to false

This indeed makes no sense. the voice_time and vp->time are not
changed in the loop, the test is either redundant or something else is
missing.

The code doesn't look right, indeed.  It's likely meant to be vp2
instead of vp.

--- a/sound/drivers/opl3/opl3_midi.c
+++ b/sound/drivers/opl3/opl3_midi.c
@@ -180,8 +180,7 @@ static int opl3_get_voice(struct snd_opl3 *opl3, int instr_4op,
  			if (vp2->state == SNDRV_OPL3_ST_ON_2OP) {
  				/* kill two voices, EXPENSIVE */
  				bp++;
-				voice_time = (voice_time > vp->time) ?
-					voice_time : vp->time;
+				voice_time = max(voice_time, vp2->time);
  			}
  		} else {
  			/* allocate 2op voice */

It's really old code, unchanged since the first git commit in 2005... Are you comfortable changing this code? One of those cases where if people didn't notice an issue in 16+ years maybe no one cares or even uses this driver...

sound/pci/lx6464es/lx_core.c:677:34: error: self-comparison always
evaluates to false

That seems like dead code indeed:

	u32 channels = runtime->channels;

	if (runtime->channels != channels)
		dev_err(chip->card->dev, "channel count mismatch: %d vs %d",
			   runtime->channels, channels);

Yes, this can be deleted.

I'll send a patch for this, thanks for the feedback.



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux