alsa-lib's new API issue (snd_ctl_elem_id_compare)

[Jaroslav Kysela]

Dne 12. 03. 21 v 2:35 Takashi Sakamoto napsal(a):
> On Thu, Mar 11, 2021 at 02:22:45PM +0100, Jaroslav Kysela wrote:
>>> Hm. I believe that you agree with the fact that we can make various
>>> algorithms to compare a pair of IDs for control elements. When focusing
>>> on fields except for numid, we can make the other algorithms against your
>>> implementation, since the ID structure is compound one. Each of the
>>> algorithms can return different result.
>>>
>>> Here, I'd like to shift the discussion to the name of new API. Although it
>>> has the most common name, 'snd_ctl_id_compare', it just has one of
>>> comparison algorithms. I have a concern that the name can gives wrong idea
>>> to users that the ID structure for control element had design to be able to
>>> be compared by itself and it would just be a single comparison algorithm.
>>>
>>> I suggest to rename the new API to express that it implements one of
>>> comparison algorithm. In a case of numid comparison, it would be
>>> 'snd_ctl_id_compare_by_numid()'. For your case,
>>> 'snd_ctl_id_compare_by_name_arithmetic' or something suitable.
>>
>> Perhaps, we can add a third argument defining the sorting algorithm, so we
>> don't bloat the symbol tables so much when we add a new sorting type (enum).
>> It would mean that the function cannot be used as a direct argument to
>> qsort(), but I think that the apps add usually an extra code to own callback
>> depending on containers, anyway. Is it more appropriate for you?
> 
> I've already investigated the idea you describe, however I concluded
> that it has more complexity than convenience.
> 
> For example, the prototype would be:
> 
> ```
> int new_api(const snd_ctl_elem_id_t *l, const snd_ctl_elem_id_t *r,
>             int (*algorithm)(const snd_ctl_elem_id_t *,
>                              const snd_ctl_elem_id_t *));
> ```
> 
> For usage with qsort_r(3), programmer should do:
> 
> ```
> int my_algo(const snd_ctl_elem_id_t *l, snd_ctl_elem_id_t *r)
> {
>    ...
> }
> 
> qsort_r(base, nmemb, size, new_api, my_algo);
> ```

I meant:

```
int new_api(const snd_ctl_elem_id_t *id1, const const snd_ctl_elem_id_t *id2,
            snd_ctl_compare_type_t ctype)
{
   ...
}

int my_algo(void *a, void *b)
{
	struct mystruct *my1 = a;
	struct mystruct *my2 = b;
	... possible extra code ...
	return new_api(&my1->id, &my2->id, SND_CTL_COMPARE_FULL_WO_NUMID);
}

qsort(base, nmemb, size, my_algo);

int my_algo_r(void *a, void *b, void *opaque)
{
	struct config *cfg = opaque;
	struct mystruct *my1 = a;
	struct mystruct *my2 = b;
	.. possibe extra code ..
	return new_api(&my1->id, &my2->id, cfg->sort_type);
}

qsort_r(base, nmemb, size, my_algo_r, cfg);
```

So I don't see a real drawback in the real use. Of course, each API has some
pros and cons, but I think that mine is easier for the common cases than the
set of functions. The two argument functions can be used directly only with
qsort() anyway.

					Jaroslav

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.