alsa-lib's new API issue (snd_ctl_elem_id_compare)

alsa-lib's new API issue (snd_ctl_elem_id_compare)

[Takashi Sakamoto]

Hi Jaroslav,

I have some concern about your commit 2cfe6addaef6[1] for alsa-lib,

It adds new library API, 'snd_ctl_elem_id_compare()', to compare a pair of
IDs for control element. In the implementation, the API call returns 0 if
they are the same, else negative or positive numeric value according to
contents of the IDs.

My concerns are:

1. evaluation of numid field is not covered.

This is not preferable since ALSA control core implementation covers two
types of comparison; numid only, and the combination iface, device,
subdevice, name, and index fields. If the API is produced for general use
cases, it should correctly handle the numid-only case, in my opinion.

2. tri-state return value is semantically inconsistent

The ID structure includes three types of values; integer, enumeration, and
string. In my opinion, tri-state return value from them has different meaning
each other. It's better just to return comparison result by boolean value,
in my opinion.

The reason I post this message instead of posting any fix is that the fix
to the API affects to alsa-utils, in which the API is used by a patch you
applied a few days ago[2]. The patch also includes change to 'AM_PATH_ALSA'
declaration in configure.ac with bumped-up version to '1.2.5', and it
disables to rebuild alsa-utils on the latest toolchain. (alsa-lib 1.2.5 is
not released yet.)

I'd like to drop the usage of API from alsa-utils with equivalent
alternative codes in alsa-utils side at first, apart from the new API in
alsa-lib so that alsa-utils can be still build on the latest toolchains to
avoid any confusion in user side. Then I'd fix the issued API carefully
so that it can be used so long as exposed API without any issue.

[1] https://github.com/alsa-project/alsa-lib/commit/2cfe6addaef6a0afa72699ec07a45e7f2fa445ba
[2] https://github.com/alsa-project/alsa-utils/commit/17b4129e6c89d1a96d4d86dabea38389927e3cf4


Regards

Takashi Sakamoto

Re: alsa-lib's new API issue (snd_ctl_elem_id_compare)

[Jaroslav Kysela]

Dne 08. 03. 21 v 13:58 Takashi Sakamoto napsal(a):
> Hi Jaroslav,
> 
> I have some concern about your commit 2cfe6addaef6[1] for alsa-lib,
> 
> It adds new library API, 'snd_ctl_elem_id_compare()', to compare a pair of
> IDs for control element. In the implementation, the API call returns 0 if
> they are the same, else negative or positive numeric value according to
> contents of the IDs.

I just noted that I didn't fill the function doc text correctly.

> 
> My concerns are:
> 
> 1. evaluation of numid field is not covered.
> 
> This is not preferable since ALSA control core implementation covers two
> types of comparison; numid only, and the combination iface, device,
> subdevice, name, and index fields. If the API is produced for general use
> cases, it should correctly handle the numid-only case, in my opinion.

My motivation was to allow to use this function for qsort() for example. The
numid and full-field comparisons are two very different things.

> 2. tri-state return value is semantically inconsistent

It's correct for the sorting (strcmp like).

> The ID structure includes three types of values; integer, enumeration, and
> string. In my opinion, tri-state return value from them has different meaning
> each other.
True, the signess is the key, the value should be ignored.

> The reason I post this message instead of posting any fix is that the fix
> to the API affects to alsa-utils, in which the API is used by a patch you
> applied a few days ago[2]. The patch also includes change to 'AM_PATH_ALSA'
> declaration in configure.ac with bumped-up version to '1.2.5', and it
> disables to rebuild alsa-utils on the latest toolchain. (alsa-lib 1.2.5 is
> not released yet.)

The latest alsa-lib in the git repo is already set to 1.2.5pre1, so if you
upgrade alsa-lib, everything should be fine. I was a bit lazy to write a
configure test and add a wrapper to alsactl to support the older alsa-lib.

					Jaroslav

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

Re: alsa-lib's new API issue (snd_ctl_elem_id_compare)

[Takashi Sakamoto]

On Mon, Mar 08, 2021 at 03:33:46PM +0100, Jaroslav Kysela wrote:
> Dne 08. 03. 21 v 13:58 Takashi Sakamoto napsal(a):
> > My concerns are:
> > 
> > 1. evaluation of numid field is not covered.
> > 
> > This is not preferable since ALSA control core implementation covers two
> > types of comparison; numid only, and the combination iface, device,
> > subdevice, name, and index fields. If the API is produced for general use
> > cases, it should correctly handle the numid-only case, in my opinion.
> 
> My motivation was to allow to use this function for qsort() for example. The
> numid and full-field comparisons are two very different things.
 
Yep. I easily associated sort implementation in hcontrol API or simple
mixer API from your implementation

However, the new API is a part of control API and it just achieves things without
any supplemental information given from userspace implementation.

> > 2. tri-state return value is semantically inconsistent
> 
> It's correct for the sorting (strcmp like).
> 
> > The ID structure includes three types of values; integer, enumeration, and
> > string. In my opinion, tri-state return value from them has different meaning
> > each other.
> True, the signess is the key, the value should be ignored.
 
Semantically, comparability is different from equality. Even if we can find
ordered pair for integer values, we can not find any ordered pair for
enumeration and strings without programmer's arbitrariness. In short, we
can not see order in SNDRV_CTL_ELEM_IFACE_CARD and
SNDRV_CTL_ELEM_IFACE_HWDEP even if in C language enumeration is an alias
to integer value.

As long as ID structure for control element is compound structure with
several types of values, it's not worse to find comparability to it for
sorting purpose.

I wish you to follow specification described in ALSA control core and
UAPI to keep consistency against semantics of IDs for control element
in this subsystem when producing low-level control API.

> > The reason I post this message instead of posting any fix is that the fix
> > to the API affects to alsa-utils, in which the API is used by a patch you
> > applied a few days ago[2]. The patch also includes change to 'AM_PATH_ALSA'
> > declaration in configure.ac with bumped-up version to '1.2.5', and it
> > disables to rebuild alsa-utils on the latest toolchain. (alsa-lib 1.2.5 is
> > not released yet.)
> 
> The latest alsa-lib in the git repo is already set to 1.2.5pre1, so if you
> upgrade alsa-lib, everything should be fine. I was a bit lazy to write a
> configure test and add a wrapper to alsactl to support the older alsa-lib.

The laziness is preferable in the most cases in our work, however in the case 
it's worse. Everything is not fine, breaking things carelessly.

At a quick glance, you've introduced below APIs since v1.2.4 release.

 * int snd_config_get_card()
 * int snd_ctl_elem_id_compare()
 * void snd_ctl_elem_info_set_read_write()
 * void snd_ctl_elem_info_set_tlv_read_write()
 * void snd_ctl_elem_info_set_inactive()

One of the above, 'snd_ctl_elem_id_compare()' is just used by
alsa-utils, and the rest is not used fortunately. I'll post patch to
alsa-utils to get back alsa-lib compatibility to v1.0.27, or better
version number.

For the above comparison API, as I described, it's not appropriate. ID
structure for control element is not comparable, thus it should be dropped
or replaced with equality function such as 'snd_ctl_elem_id_equal()'.

When you need any sorting algorithms, it should be implemented in
application side or alsa-lib API in the other layer such as hcontrol and
simple mixer since control API should follow to specification of ALSA
control written in kernel land.


Thanks

Takashi Sakamoto

Re: alsa-lib's new API issue (snd_ctl_elem_id_compare)

[Jaroslav Kysela]

Dne 09. 03. 21 v 1:38 Takashi Sakamoto napsal(a):
> On Mon, Mar 08, 2021 at 03:33:46PM +0100, Jaroslav Kysela wrote:
>> Dne 08. 03. 21 v 13:58 Takashi Sakamoto napsal(a):
>>> My concerns are:
>>>
>>> 1. evaluation of numid field is not covered.
>>>
>>> This is not preferable since ALSA control core implementation covers two
>>> types of comparison; numid only, and the combination iface, device,
>>> subdevice, name, and index fields. If the API is produced for general use
>>> cases, it should correctly handle the numid-only case, in my opinion.
>>
>> My motivation was to allow to use this function for qsort() for example. The
>> numid and full-field comparisons are two very different things.
>  
> Yep. I easily associated sort implementation in hcontrol API or simple
> mixer API from your implementation
> 
> However, the new API is a part of control API and it just achieves things without
> any supplemental information given from userspace implementation.

It's not required, if documented. Nobody forces to use this function in the
app code.

> For the above comparison API, as I described, it's not appropriate. ID
> structure for control element is not comparable, thus it should be dropped
> or replaced with equality function such as 'snd_ctl_elem_id_equal()'.

I don't require the numid match at this point. The numid is not known or may
change for the id entered by the user. So I need to forcefully ignore it.

If we need a function which follow numid + full id comparison, then we can
introduce it. I agree that it should return only a boolean return value in
this case.

> When you need any sorting algorithms, it should be implemented in
> application side or alsa-lib API in the other layer such as hcontrol and
> simple mixer since control API should follow to specification of ALSA
> control written in kernel land.

I don't follow your arguments here. The numid and full field comparisons are
two different things. The caller must know things behind the scene.
The snd_ctl_elem_id_compare() function may be used as a quick backend for the
full field comparisons with the optimized execution (reduce app -> library calls).

The enums conversion to integers: I think that we're safe here. The interface
enum numbers cannot change and we know the range (and the order), so it's safe.

Lastly, the qsort() with snd_ctl_id_compare() as an argument will produce a
consistent, understandable result, right?

					Jaroslav

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

Re: alsa-lib's new API issue (snd_ctl_elem_id_compare)

[Takashi Sakamoto]

Hi,

Sorry to be late for reply but I have a bit busy for patchset to test
programs of axfer in alsa-utils[1].

On Tue, Mar 09, 2021 at 01:37:26PM +0100, Jaroslav Kysela wrote:
> Dne 09. 03. 21 v 1:38 Takashi Sakamoto napsal(a):
> > On Mon, Mar 08, 2021 at 03:33:46PM +0100, Jaroslav Kysela wrote:
> >> Dne 08. 03. 21 v 13:58 Takashi Sakamoto napsal(a):
> >>> My concerns are:
> >>>
> >>> 1. evaluation of numid field is not covered.
> >>>
> >>> This is not preferable since ALSA control core implementation covers two
> >>> types of comparison; numid only, and the combination iface, device,
> >>> subdevice, name, and index fields. If the API is produced for general use
> >>> cases, it should correctly handle the numid-only case, in my opinion.
> >>
> >> My motivation was to allow to use this function for qsort() for example. The
> >> numid and full-field comparisons are two very different things.
> >  
> > Yep. I easily associated sort implementation in hcontrol API or simple
> > mixer API from your implementation
> > 
> > However, the new API is a part of control API and it just achieves things without
> > any supplemental information given from userspace implementation.
> 
> It's not required, if documented. Nobody forces to use this function in the
> app code.
>
> > For the above comparison API, as I described, it's not appropriate. ID
> > structure for control element is not comparable, thus it should be dropped
> > or replaced with equality function such as 'snd_ctl_elem_id_equal()'.
> 
> I don't require the numid match at this point. The numid is not known or may
> change for the id entered by the user. So I need to forcefully ignore it.
> 
> If we need a function which follow numid + full id comparison, then we can
> introduce it. I agree that it should return only a boolean return value in
> this case.
> 
> > When you need any sorting algorithms, it should be implemented in
> > application side or alsa-lib API in the other layer such as hcontrol and
> > simple mixer since control API should follow to specification of ALSA
> > control written in kernel land.
> 
> I don't follow your arguments here. The numid and full field comparisons are
> two different things. The caller must know things behind the scene.
> The snd_ctl_elem_id_compare() function may be used as a quick backend for the
> full field comparisons with the optimized execution (reduce app -> library calls).
> 
> The enums conversion to integers: I think that we're safe here. The interface
> enum numbers cannot change and we know the range (and the order), so it's safe.
> 
> Lastly, the qsort() with snd_ctl_id_compare() as an argument will produce a
> consistent, understandable result, right?

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.


Thanks

[1] https://mailman.alsa-project.org/pipermail/alsa-devel/2021-March/181947.html

Takashi Sakamoto

Re: alsa-lib's new API issue (snd_ctl_elem_id_compare)

[Jaroslav Kysela]

Dne 11. 03. 21 v 13:46 Takashi Sakamoto napsal(a):
> Hi,
> 
> Sorry to be late for reply but I have a bit busy for patchset to test
> programs of axfer in alsa-utils[1].
> 
> On Tue, Mar 09, 2021 at 01:37:26PM +0100, Jaroslav Kysela wrote:
>> Dne 09. 03. 21 v 1:38 Takashi Sakamoto napsal(a):
>>> On Mon, Mar 08, 2021 at 03:33:46PM +0100, Jaroslav Kysela wrote:
>>>> Dne 08. 03. 21 v 13:58 Takashi Sakamoto napsal(a):
>>>>> My concerns are:
>>>>>
>>>>> 1. evaluation of numid field is not covered.
>>>>>
>>>>> This is not preferable since ALSA control core implementation covers two
>>>>> types of comparison; numid only, and the combination iface, device,
>>>>> subdevice, name, and index fields. If the API is produced for general use
>>>>> cases, it should correctly handle the numid-only case, in my opinion.
>>>>
>>>> My motivation was to allow to use this function for qsort() for example. The
>>>> numid and full-field comparisons are two very different things.
>>>  
>>> Yep. I easily associated sort implementation in hcontrol API or simple
>>> mixer API from your implementation
>>>
>>> However, the new API is a part of control API and it just achieves things without
>>> any supplemental information given from userspace implementation.
>>
>> It's not required, if documented. Nobody forces to use this function in the
>> app code.
>>
>>> For the above comparison API, as I described, it's not appropriate. ID
>>> structure for control element is not comparable, thus it should be dropped
>>> or replaced with equality function such as 'snd_ctl_elem_id_equal()'.
>>
>> I don't require the numid match at this point. The numid is not known or may
>> change for the id entered by the user. So I need to forcefully ignore it.
>>
>> If we need a function which follow numid + full id comparison, then we can
>> introduce it. I agree that it should return only a boolean return value in
>> this case.
>>
>>> When you need any sorting algorithms, it should be implemented in
>>> application side or alsa-lib API in the other layer such as hcontrol and
>>> simple mixer since control API should follow to specification of ALSA
>>> control written in kernel land.
>>
>> I don't follow your arguments here. The numid and full field comparisons are
>> two different things. The caller must know things behind the scene.
>> The snd_ctl_elem_id_compare() function may be used as a quick backend for the
>> full field comparisons with the optimized execution (reduce app -> library calls).
>>
>> The enums conversion to integers: I think that we're safe here. The interface
>> enum numbers cannot change and we know the range (and the order), so it's safe.
>>
>> Lastly, the qsort() with snd_ctl_id_compare() as an argument will produce a
>> consistent, understandable result, right?
> 
> 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?

						Jaroslav

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

Re: alsa-lib's new API issue (snd_ctl_elem_id_compare)

[Takashi Sakamoto]

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);
```

On the other hand, the API has name to express itself appropriately and
we have some of such APIs:

```
int the_api_by_algo_a(const snd_ctl_elem_id_t *l,
                      const snd_ctl_elem_id_t *r);
int the_api_by_algo_b(const snd_ctl_elem_id_t *l,
                      const snd_ctl_elem_id_t *r);
int the_api_by_algo_c(const snd_ctl_elem_id_t *l,
                      const snd_ctl_elem_id_t *r);
...
```

The programmer selects one of them, then:

```
qsort(base, nmemb, size, the_api_by_algo_a);
```

Or select one of them dynamically if need:

```
int (*algo)(const snd_ctl_elem_id_t *, const snd_ctl_elem_id_t *);

switch (cond) {
case A:
    algo = the_api_by_algo_a;
    break;
case B:
    algo = the_api_by_algo_b;
    break;
case C:
    algo = the_api_by_algo_c;
    break;
default:
    return -EINVAL;
}

qsort(base, nmemb, size, algo);
```

For the case of hctl/mixer container about which you mentioned,
qsort_r(3) style is convenient for the case that programmer need to
re-implement own comparison algorithm. However the decision is still
up to the programmer, in short:

```
int my_algo(const snd_ctl_elem_id_t *l,
            const snd_ctl_elem_id_t *r,
            void *arg);
qsort_r(base, nmemb, size, my_algo, my_arg);
```

Here, I think it more worth to share algorithms than keeping less entries
in symbol table in shared library. Just the thought of it, I can devise
some algorithms below:

 * by numid
 * by name arithmetic (=your implementation)
 * by the words 'playback' and 'capture', case-insensitive or sensitive
 * by device and subdevice


Regards

Takashi Sakamoto

Re: 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@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: alsa-lib's new API issue (snd_ctl_elem_id_compare)

[Takashi Sakamoto]

Hi,

On Fri, Mar 12, 2021 at 11:09:42AM +0100, Jaroslav Kysela wrote:
> 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,

Yep. In alsa-utils, you just use the new API to check equality of a pair
of IDs for control element. At present, there are no codes to call the
new API for sorting purpose.

> but I think that mine is easier for the common cases than the set of functions.

Although I agree that your implementation for the comparison algorithm is
useful, it's just one of some useful algorithms. It is difficult to
handle your algorithm as the most valuable, specific, unique one apart from
the other ones. The name, 'snd_ctl_elem_id_compare' is inappropriate for the
case, since it could exclude possibility to implement the other useful
algorithms.

Actually you need to describe the exclusiveness into the function comment,
"The numid comparison can be easily implemented using snd_ctl_elem_id_get_numid()
calls."[1]. And it interfaces to produce comparison algorithm by numid field as
optimized function (=done by single function call, without two function call
to get numid from opaque pointers).

> The two argument functions can be used directly only with qsort() anyway.

At present, it's a better compromise to implement the set of functions
exposed to applications for each algorithm, I think. When we got many
functions, then start to discuss about the unified-style function for
sorting.

As a first step, let me rename your implementation, then add another
implementation to compare numid field. What do you think about it?


[1] https://github.com/alsa-project/alsa-lib/commit/266618088aa6e17672ffb08a110b2fff2e2f7ad2

Regards

Takashi Sakamoto