seq: snd_seq_parse_address() accepts port name

seq: snd_seq_parse_address() accepts port name

[GitHub pull_request - opened]

alsa-project/alsa-lib pull request #528 was opened from thanks4opensource:

This pull requests allows `snd_seq_parse_address()` to accept port names as well as port id numbers. This improves the usability of `aconnect`, particularly within shell scripts where port names are more readable/self-documenting than port numbers, and are also more robust as client updates are more likely to reorder/renumber ports than to change their names.

The functionality could be added within `aconnect` itself, but doing so inside alsa-lib makes it available for all programs, e.g. ones that use `snd_seq_subscribe_port()`,etc to connect themselves to other clients.

The new code is almost completely compatible with existing use-cases, with one exception: If passed a non-numeric port, a bug in the current `snd_seq_parse_address()` sets 0 for the port id/number without returning an error. This causes `aconnect` to silently make an incorrect connection or fail with a confusing error message.

Consider the following example:

```
$ aconnect -oil
client 128: 'arbitrary_client' [type=user,pid=837984]
    0 'output          '
    1 'input           '
client 129: 'synthesizer' [type=user,pid=837982]
    0 'outside_speaker_input'
    1 'headphone_input '
client 130: 'virtual_piano' [type=user,pid=837983]
    0 'output          '
```
Until I looked at the source code, I never understood:

```
$ aconnect 130 128:100
Connection failed (Invalid argument)
$ aconnect 130 128:input
Connection failed (Operation not permitted)
$ aconnect 130 128:1 
$ aconnect -oil     
client 128: 'arbitrary_client' [type=user,pid=837984]
    0 'output          '
    1 'input           '
	Connected From: 130:0
client 129: 'synthesizer' [type=user,pid=837982]
    0 'outside_speaker_input'
    1 'headphone_input '
client 130: 'virtual_piano' [type=user,pid=837983]
    0 'output          '
	Connecting To: 128:1
```

If the error message had been "Invalid argument" instead of "Operation not permitted" (why can't I connect an output to an input?) it would have been less of a mystery.

Equally confusing, and potentially dangerous:

```
$ aconnect -x
$ aconnect 130 129:headphone_input
$ aconnect -oil                   
client 128: 'arbitrary_client' [type=user,pid=838343]
    0 'output          '
    1 'input           '
client 129: 'synthesizer' [type=user,pid=838347]
    0 'outside_speaker_input'
	Connected From: 130:0
    1 'headphone_input '
client 130: 'virtual_piano' [type=user,pid=838350]
    0 'output          '
	Connecting To: 129:0
```

That's a somewhat contrived example, but imagine it's 3 AM and you want to practice some loud heavy metal keyboard riffs. Suddenly you're in big trouble with your neighbors. ;)

This pull request "does the right thing" in all cases. My general policy is to never change existing behavior, but in this case the "silently convert to port 0" is at best an undocumented feature, and at worst an undeniable bug. Anything relying on it really should be fixed.

The code maintains current feature of accepting a prefix of a client name ("FLU" instead of "FLUID Synth"). If accepts any substring for port names because several common ALSA clients have long port names which differ only at their ends ("alt" for "Synth input alternate"). In both cases an exact name match overrides a prefix/substring.

The branch includes test/address.c which extensively validates `snd_seq_parse_address()` (4349 tests at latest count), including checking returned errors in cases of nonexistent client or port names, out of range client/port id numbers, etc. As necessitated by the sequencer API, and as per the current code, it requires a non-NULL `snd_seq_t*` argument to handle names but will parse purely numeric ids without. It currently does not check whether there are existing clients/ports matching the requested client/port numbers.

I apologize for completely rewriting `snd_seq_parse_address()`, but the existing version parsed the port string before the client, including multiple early returns from the function. The client is required for searching port names, and the code would have been a mess if the order of operations had been retained. Of course I also believe the new version, particularly the parsing of the input string into client and port, is simpler and more readable.

Please consider merging this pull request. Thank you.

Request URL   : https://github.com/alsa-project/alsa-lib/pull/528
Patch URL     : https://github.com/alsa-project/alsa-lib/pull/528.patch
Repository URL: https://github.com/alsa-project/alsa-lib