[alsa-devel] [alsa-lib][PATCH 3/4] pcm: linear, route: handle linear formats with 20-bit sample on 4 bytes

[Maciej S. Szmigiero]

On 05.12.2017 15:57, Takashi Iwai wrote:
> On Tue, 05 Dec 2017 15:37:09 +0100,
> Takashi Sakamoto wrote:
>>
>> Hi,
>>
(..)
>> In my opinion, addition of constraint of PCM hardware parameters to
>> these PCM plugins is preferable to insertion of the assertions,
>> because abort inner library is bad idea itself. Proper errors should
>> be returned
>> to applications in protocol via alsa-lib PCM APIs. It's more friendly to
>> usual developers and users.
> 
> Well, it's basically moot to discuss about it, as
> snd_pcm_lib_linear_get_index() is an internal function, i.e. it's not
> supposed to be called from applications.
> 
> In that sense, assert() wouldn't be too bad.   However, the primary
> question is -- would it really catch anything in practice?  What drove
> you to put that assert() at all?
> 
> If a check of the valid formats, then calling snd_pcm_format_linear()
> is the safest.

The reason I added these asserts is that snd_pcm_linear_{get,put}_index()
use result from snd_pcm_format_width() directly to calculate an array
index.
If, for any reason, this function returns an unexpected width a subtle
bug will emerge.

Three reasons that I could think of why an unexpected width would be
returned is a bug in a plugin that uses these functions (currently 9
plugins use them) that fail to properly restrict source / destination
format space to linear formats only, a bug in general format refining
code or somebody adding a format to linear formats mask without
adapting conversion arrays, too.

I know that these may sound like rare occurrences but since this
check is close to zero-cost (it only happens on plugin stack
initialization time and not, for example, at every sample) I think it
is worth the extra time (and one can always compile the library with
NDEBUG to skip it).

> thanks,
> 
> Takashi
> 

Thanks,
Maciej