Re: ALSA: firewire-lib: schedule work again when MIDI substream has rest of MIDI messages

Re: ALSA: firewire-lib: schedule work again when MIDI substream has rest of MIDI messages

[Dan Carpenter]

Hello Takashi Sakamoto,

This is a semi-automatic email about new static checker warnings.

The patch e8a40d9bcb23: "ALSA: firewire-lib: schedule work again when 
MIDI substream has rest of MIDI messages" from Oct 9, 2015, leads to 
the following Smatch complaint:

sound/firewire/lib.c:88 async_midi_port_callback()
	 error: we previously assumed 'substream' could be null (see line 77)

sound/firewire/lib.c
    76	
    77		if (rcode == RCODE_COMPLETE && substream != NULL)
                                               ^^^^^^^^^^^^^^^^^
Existing code assumes substream can be NULL.

    78			snd_rawmidi_transmit_ack(substream, port->consume_bytes);
    79		else if (!rcode_is_permanent_error(rcode))
    80			/* To start next transaction immediately for recovery. */
    81			port->next_ktime = ktime_set(0, 0);
    82		else
    83			/* Don't continue processing. */
    84			port->error = true;
    85	
    86		port->idling = true;
    87	
    88		if (!snd_rawmidi_transmit_empty(substream))
                                                ^^^^^^^^^
Patch adds an unchecked dereference (inside the function call).

    89			schedule_work(&port->work);
    90	}

regards,
dan carpenter

Re: ALSA: firewire-lib: schedule work again when MIDI substream has rest of MIDI messages

[Takashi Sakamoto]

Hi,

On Oct 15 2015 16:04, Dan Carpenter wrote:
> Hello Takashi Sakamoto,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch e8a40d9bcb23: "ALSA: firewire-lib: schedule work again when
> MIDI substream has rest of MIDI messages" from Oct 9, 2015, leads to
> the following Smatch complaint:
>
> sound/firewire/lib.c:88 async_midi_port_callback()
> 	 error: we previously assumed 'substream' could be null (see line 77)
>
> sound/firewire/lib.c
>      76	
>      77		if (rcode == RCODE_COMPLETE && substream != NULL)
>                                                 ^^^^^^^^^^^^^^^^^
> Existing code assumes substream can be NULL.
>
>      78			snd_rawmidi_transmit_ack(substream, port->consume_bytes);
>      79		else if (!rcode_is_permanent_error(rcode))
>      80			/* To start next transaction immediately for recovery. */
>      81			port->next_ktime = ktime_set(0, 0);
>      82		else
>      83			/* Don't continue processing. */
>      84			port->error = true;
>      85	
>      86		port->idling = true;
>      87	
>      88		if (!snd_rawmidi_transmit_empty(substream))
>                                                  ^^^^^^^^^
> Patch adds an unchecked dereference (inside the function call).
>
>      89			schedule_work(&port->work);
>      90	}
>
> regards,
> dan carpenter

Indeed. I'll fix this bug in this weekend.


Thanks

Takashi Sakamoto