Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: >>>> Gilles Chanteperdrix wrote: >>>>> Jan Kiszka wrote: >>>>>> Gilles Chanteperdrix wrote: >>>>>>> Jan Kiszka wrote: >>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>> Jan Kiszka wrote: >>>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>>> Jan Kiszka wrote: >>>>>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>>>>> GIT version control wrote: >>>>>>>>>>>>>> Module: xenomai-jki >>>>>>>>>>>>>> Branch: for-upstream >>>>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>>>> URL: >>>>>>>>>>>>>> http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>>>> >>>>>>>>>>>>>> Author: Jan Kiszka <jan.kis...@siemens.com> >>>>>>>>>>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>>>>>>>>>> >>>>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>>>>>>>>>> >>>>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get >>>>>>>>>>>>>> initialized when using > 1 skin libraries. >>>>>>>>>>>>> If that would be possible, then it is the conversion variables >>>>>>>>>>>>> which >>>>>>>>>>>>> should made be weak, not the function. >>>>>>>>>>>>> >>>>>>>>>>>>> The way I see it, the posix and native skins currently get a >>>>>>>>>>>>> different >>>>>>>>>>>>> set of variables and functions, which works, but with your >>>>>>>>>>>>> change, since >>>>>>>>>>>>> there is only one function, only one set of variable gets >>>>>>>>>>>>> initialized by >>>>>>>>>>>>> the two function calls. And one skin just broke. >>>>>>>>>>>>> >>>>>>>>>>>>> Or am I missing something? Does the patch fix a problem you >>>>>>>>>>>>> really had? >>>>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the >>>>>>>>>>>> libs >>>>>>>>>>>> there is non-trivial. But I was able to observe that only one set >>>>>>>>>>>> of >>>>>>>>>>>> functions is used - which is logical considering the weak marks. >>>>>>>>>>>> And >>>>>>>>>>>> this breaks due to the static inline initialization. >>>>>>>>>>>> >>>>>>>>>>>> However, let's mark both functions and variables weak to fix the >>>>>>>>>>>> issue >>>>>>>>>>>> and avoid leaving unused variables around. Will update my patch in >>>>>>>>>>>> a minute. >>>>>>>>>>> Ok. I am reverting this patch until you provide me with another >>>>>>>>>>> solution. It causes latency to segfault purely and simply at >>>>>>>>>>> startup on >>>>>>>>>>> my dual PIII. >>>>>>>>>>> >>>>>>>>>> Cannot reproduce yet. Do you have a backtrace? >>>>>>>>> No. But the problem is probably the same as the one signaled by Henri, >>>>>>>>> a misplaced weak directive ending up in a symbol with no address at >>>>>>>>> all. >>>>>>>>> Since the current situation works, I am going to wait for the "clean" >>>>>>>>> fix which puts some code/data in the src/skins/common directory. >>>>>>>>> >>>>>>>> Find it in my tree. But it's not yet well tested. >>>>>>> I do not like it either. Functions which are in src/skins/common should >>>>>>> still be weak, since this lib is included in all the skins libraries. >>>>>> Those functions are now in libxeno_common only, so I see no point in >>>>>> allowing them to be overloaded. >>>>> Yes, but libxeno_common is included in libpthread_rt.so and >>>>> libnative.so. So, if you link with both libraries, you get >>>>> libxeno_common twice. >>>>> >>>> Do we link libxeno_common statically? Otherwise, this conflict is not >>>> logical to me. Also, there are other symbols in bind.c that are non-weak. >>> libxeno_common is a "convenience library", which means that when >>> libpthread_rt.so is assembled, the object files from libxeno_common are >>> included. >> BTW, what speaks against making it a dynamic library? > > Complications. Currently the way of linking a native application is to do: > > `xeno-config --ldflags` -lnative > > Since when linking, the order matter, we can not hide this new library > in xeno-config --ldflags, so we would have to turn this into: > > `xeno-config --ldflags` -lnative -lxeno-common > > i.e. breaking user way of building applications. I do not think the gain > is worth the trouble: linking against only one xenomai library looks to > me like the most common case.
What would break when swapping -lnative and -xeno-common arbitrarily? There is a _lot_ of cleanup potential. E.g. all that redundant __real wrappers are good candidates for libxeno-common. > > >>> I guess the loader eliminates the duplicates. >>> >> It has to. For the same reason, we should be able to clean up >> skins/common/current.c now. > > I am not that confident about that change. By using the weak directive, > we make sure that the same __thread variable is used for instance. If we > do not do that, what prevents the code from using the local symbols in > each library ? IOW, removing the duplicate could work for libxeno_common > symbols when invoked externally (which probably almost never happens), > but not inside the skin libraries. I do not know about that, which is > why I kept the weak declarations. Mixing weak functions with non-weak variables can cause troubles as well. Sticking our head into the sand is no good approach, understanding the requirements for weak or not is better - and eliminating weak would be best. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux _______________________________________________ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
