[alsa-devel] [PATCH] Emit an error message when dlopen fails.

[Takashi Sakamoto]

Hi Jaroslav,

On ov 22 2017 19:13, Jaroslav Kysela wrote:
> Dne 22.11.2017 v 10:30 Takashi Sakamoto napsal(a):
>> On Nov 22 2017 16:45, Cedric Roux wrote:
>>> Ah, this is a problem.
>>> But I struggled with the "alsaeq plugin" which, under ubuntu 17 something
>>> is not linked against libasound and when pulseaudio is configured to use
>>> it, it fails without saying anything useful and the hacker (me) has to spend too
>>> much time because ALSA does not use dlerror.
>>>
>>>>
>>>> (parsing alsa.conf)
>>>> snd_config_hooks_call()
>>>>     ->snd_dlopen(name = NULL)
>>>>       if (name == NULL)
>>>>         name = self
>>>>       filename = ALSA_PLUGIN_DIR + '/' + name
>>>>       dlopen(filename)
>>>>       (generate the error messages)
>>>>
>>>> I don't know why alsa-lib is programmed as what it is, so have no idea to suggest better solution...
>>>>
>>>> Additionally, your patch includes some issues of patch format:
>>>> * Not only your signature but also patch comment is required. The first
>>>>     line is interpreted as patch title, and the rest is as comment.
>>>
>>> okay, next time I'll write some text.
>>>
>>>> * Patches should be compliant to git-patch format. I encountered failure
>>>>     of 'git am' for this patch. Please use 'git format-patch' command to
>>>>     generate patches for posting.
>>>
>>> I used format-patch and imap-send. Maybe this bloated thunderbird did
>>> mess up (I configured it as explained in the man pages).
>>
>> Aha. You did use git-format-patch to generate it.
>>
>> When I generate patches by 'git format-patch', code differences are
>> output with 'diff --git a/src/dlmisc.c b/src/dlmisc.c'. On the other
>> hand, your patch includes 'diff --git src/dlmisc.c src/dlmisc.c'. As
>> long as asking my colleague, this seems to come from '--no-prefix'
>> option for 'git-format-patch'. This is a cause of my failure to run 'git
>> am' because an additional option, '-p1' is required to apply your patch.
>> Now I got it.
>>
>>> Anyway, do what you want with the patch above, I don't really care actually.
>>> Problem is solved on my side. I've seen messages in the wild from people
>>> with the same problem, I thought I could help a bit. I didn't see the
>>> "big picture" (that is: bad design of too many software).
>>>
>>> All I know is that it's good practice to report errors to users. If
>>> a code path can "legitimately" generate errors, then the programmer
>>> needs to deal with it in a convenient way.
>>>
>>> But yeah, I don't care. ALSA lib is bloated to start with.
>>> (This email is my last, I won't troll more, promise.)
>>
>> Current implementation of alsa-lib is too bloated to me as well. But I'd
>> like to avoid people encountering the similar issues. This is my
>> motivation to tackle such a huge, complicated and troublesome library.
> 
> We should return the error string from snd_dlopen() in this case. It's
> up to the caller to handle this (ignore or print) correctly.
> 
> I will implement it.

I had the same idea and in my opinion it's more convenient than 
Iwai-san's suggestion (using local SNDMSG macro). Feel free to add me to 
CC list when posting your patch for it. I'm willing to review it ;)


Thanks

Takashi Sakamoto