Re: [bug report] ALSA: ump: Add legacy raw MIDI support

[Takashi Iwai <tiwai@suse.de>]

On Thu, 25 May 2023 10:00:10 +0200,
Dan Carpenter wrote:
> 
> Hello Takashi Iwai,
> 
> The patch 0b5288f5fe63: "ALSA: ump: Add legacy raw MIDI support" from
> May 23, 2023, leads to the following Smatch static checker warning:
> 
> 	sound/core/ump_convert.c:460 do_convert_to_ump()
> 	warn: right shifting more than type allows 8 vs 8
> 
> sound/core/ump_convert.c
>     419 static int do_convert_to_ump(struct snd_ump_endpoint *ump,
>     420                              unsigned char group, unsigned char c, u32 *data)
>     421 {
>     422         /* bytes for 0x80-0xf0 */
>     423         static unsigned char cmd_bytes[8] = {
>     424                 3, 3, 3, 3, 2, 2, 3, 0
>     425         };
>     426         /* bytes for 0xf0-0xff */
>     427         static unsigned char system_bytes[16] = {
>     428                 0, 2, 3, 2, 0, 0, 1, 0, 1, 1, 1, 1, 0, 0, 1, 1
>     429         };
>     430         struct ump_cvt_to_ump *cvt = &ump->out_cvts[group];
>     431         unsigned char bytes;
>     432 
>     433         if (c == UMP_MIDI1_MSG_SYSEX_START) {
>     434                 cvt->in_sysex = 1;
>     435                 cvt->len = 0;
>     436                 return 0;
>     437         }
>     438         if (c == UMP_MIDI1_MSG_SYSEX_END) {
>     439                 if (!cvt->in_sysex)
>     440                         return 0; /* skip */
>     441                 return cvt_legacy_sysex_to_ump(cvt, group, data, true);
>     442         }
>     443 
>     444         if ((c & 0xf0) == UMP_MIDI1_MSG_REALTIME) {
>     445                 bytes = system_bytes[c & 0x0f];
>     446                 if (!bytes)
>     447                         return 0; /* skip */
>     448                 if (bytes == 1) {
>     449                         data[0] = ump_compose(UMP_MSG_TYPE_SYSTEM, group, 0, c);
>     450                         return 4;
>     451                 }
>     452                 cvt->buf[0] = c;
>     453                 cvt->len = 1;
>     454                 cvt->cmd_bytes = bytes;
>     455                 cvt->in_sysex = 0; /* abort SysEx */
>     456                 return 0;
>     457         }
>     458 
>     459         if (c & 0x80) {
> --> 460                 bytes = cmd_bytes[(c >> 8) & 7];
>                                            ^^^^^^
> Based on the if statement, it looks like c >> 4 was intended.

Right, currently it's a bogus value.  I'll submit the fix patch.

Thanks!

Takashi