[alsa-devel] [PATCH] amidi: add delay option

[Felipe Ferreri Tonello]

Hi Clemens,

On 12/08/16 08:10, Clemens Ladisch wrote:
> Felipe F. Tonello wrote:
>> This patch adds a new options to amidi tool to add a delay (in milliseconds)
>> to each MIDI message.
>>
>> This is very useful when sending firmware updates to a remote device via SysEx
>> or any other use that requires this delay in between messages.
> 
>> +	case 0xf4:
>> +	case 0xf5:
>> +		return 0; /* ignore */
>> +	case 0xf7:
>> +		return 0; /* ignore */
> 
> Silent data loss is bad.

What do you mean by that?

> 
>> +	case 0x00 ... 0x7f:
>> +		return rstate_len ?: rstate_len - 1;
> 
> rstate_len - 1 == -1

?

That's why there is this condition, if rstate_len is zero, than it
returns 0.

> 
>> +				int len = message_length(*data);
>> +
>> +				/* check for variable length */
>> +				if (len == -1) {
>> +					int i = 0;
>> +					while (data[i++] != 0xf7);
> 
> Possible buffer overflow.

True. I could check while data + i < send_data + send_data_length.

> 
>> +				if (len > 0 &&
>> +				    (err = snd_rawmidi_write(output, data, len)) < 0) {
> 
> Same here.

This would be solved by previous boundaries check.

> 
> 
> The most robust way to find message boundaries probably would be a state
> machine.  However, there is actually no requirement to find all message
> boundaries; the _actual_ requirement is to insert pauses between SysEx
> messages.  So don't bother to try to parse everything; just search for
> F0 or F7.

Well, I tough it would be useful to work in any type of MIDI message not
just SysEx. So, do you think only SysEx then?

-- 
Felipe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0x92698E6A.asc
Type: application/pgp-keys
Size: 7195 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20160812/ce22f950/attachment.bin>