[alsa-devel] ["PATCH alsa-lib"] Change snd_dlopen() function to return the error string

[Takashi Iwai]

On Tue, 28 Nov 2017 08:26:09 +0100,
Jaroslav Kysela wrote:
> 
> Dne 27.11.2017 v 22:24 Takashi Iwai napsal(a):
> > On Mon, 27 Nov 2017 21:49:06 +0100,
> > Jaroslav Kysela wrote:
> >>
> >> Dne 27.11.2017 v 21:24 Takashi Iwai napsal(a):
> >>> On Mon, 27 Nov 2017 19:19:51 +0100,
> >>> Takashi Sakamoto wrote:
> >>>>
> >>>> Hi Jaroslav,
> >>>>
> >>>> On Nov 27 2017 17:50, Jaroslav Kysela wrote:
> >>>>> The dlopen() function might fail also for another reason than
> >>>>> a missing file, thus return the error string from dlerror().
> >>>>>
> >>>>> Signed-off-by: Jaroslav Kysela <perex at perex.cz>
> >>>>> ---
> >>>>>   include/global.h        |  2 +-
> >>>>>   src/Versions.in         |  5 +++++
> >>>>>   src/conf.c              | 13 ++++++-----
> >>>>>   src/dlmisc.c            | 60 +++++++++++++++++++++++++++++++++++++++----------
> >>>>>   src/hwdep/hwdep.c       |  6 ++---
> >>>>>   src/mixer/simple_abst.c | 10 ++++-----
> >>>>>   src/pcm/pcm_hooks.c     |  8 +++----
> >>>>>   src/pcm/pcm_meter.c     |  6 ++---
> >>>>>   src/rawmidi/rawmidi.c   |  6 ++---
> >>>>>   src/seq/seq.c           |  6 ++---
> >>>>>   src/timer/timer.c       |  6 ++---
> >>>>>   src/timer/timer_query.c |  6 ++---
> >>>>>   12 files changed, 88 insertions(+), 46 deletions(-)
> >>>>
> >>>> Unfortunately, this patch brings build error in my environment (Ubuntu
> >>>> 17.10 amd64, gcc7.2.0 from gcc-7 7.2.0-8ubuntu3).
> >>>>
> >>>> $ ./gitcompile
> >>>> $ make
> >>>> make[2]: Entering directory '/tmp/alsa-lib/src'
> >>>>   CCLD     libasound.la
> >>>> /usr/bin/ld: unable to find version dependency `ALSA_1.1.6'
> >>>> pcm/.libs/libpcm.a(pcm_generic.o): In function `snd1_pcm_generic_hwsync':
> >>>> /tmp/alsa-lib/src/pcm/pcm_generic.c:142: warning:
> >>>> collect2: error: ld returned 1 exit status
> >>>> Makefile:491: recipe for target 'libasound.la' failed
> >>>> make[2]: *** [libasound.la] Error 1
> >>>>
> >>>> At present, I don't know exactly the reason, but I'll start
> >>>> investigating it in next morning (I'd like to have sleep...).
> >>>
> >>> It's likely a bogus definition in version symbols.
> >>> It has to be a form like:
> >>>
> >>>   ALSA_1.1.6 {
> >>>     global:
> >>>       @SYMBOL_PREFIX at snd_dlopen;
> >>>   } ALSA_1.1.5;
> >>>
> >>> while the patch put "ALSA_1.1.6" to both places.
> >>
> >> Yes, I forgot to modify src/Versions.in (I modified only src/Versions
> >> for my local build).
> >>
> >>> Besides that, I'm scared by the versioned symbols, of which I have
> >>> only a bad memory.
> >>
> >> I know, but the major issue that we forgot to apply old symbol versions
> >> to some functions in the past, right ? I don't see any drawback.
> > 
> > IIRC, some apps didn't compile the stuff properly with the versioned
> > symbols, thus fall back to bind with the old symbols without error,
> > which leads to a crash at runtime.
> 
> It's probably a linking issue which should be resolved by the developer.
>
> > Another case is when an app or a library does symbol resolution by
> > itself via dlopen and dlsym.  libao does it, for example.
> 
> The developers might use dlvsym() or recompile and fix compilation
> errors for new library.

Both cases can be addressed in the application side, yes.
But the problem is that it *requires* the fix in application side.

We have to provide both old and new APIs in anyway, no matter whether
versioned symbol is used or not.  Then what's the merit of the
versioned symbol, after all...?


> > The snd_dlopen() isn't supposed to be used widely in external apps, so
> > maybe the influence is limited.  But I still feel bad with the usage
> > of versioned symbols unless really needed.  In this case, we can avoid
> > the issue just by adding another better function.  And, the purpose is
> > only for a better debuggability, so the change in caller side isn't
> > mandatory.
> 
> Yep, snd_dlopen() is probably not used outside alsa-lib at all.
> 
> The glibc is full of versioned symbols and it seems that it plays good
> with them.

glibc functions are not referred with dlopen/dlsym, so the issue above
doesn't happen.  But we're in a different situation, and the versioned
symbols make thing more complicated than needed, IMO.


thanks,

Takashi