Message ID | 1412284150-11771-3-git-send-email-jengelh@inai.de |
---|---|
State | Rejected |
Headers | show |
On Thu, Oct 02, 2014 at 11:09:07PM +0200, Jan Engelhardt wrote: Good Morning Jan, > The link stage fails at some point. libosmogsm.so:lapd-core.c uses > talloc_free, but does not link to libtalloc.so. Correct this. my position has not changed. The --disable-talloc is an option for Embedded Firmware (e.g. OsmocomBB) that do not want to include talloc. It is not for people that want to use their distributions talloc. Using the osmocom/core/talloc.h and linking against another/unknown version of talloc is scraming for problems (API/ABI issues, etc.) I agree that having a talloc code clone inside libosmocore was not a good idea from us and that we should fix it in the long run. But this means _only_ requiring an external talloc, killing the osmocom/core/talloc.h include file. But that is different to the patches you propose. For this reason I am not taking your talloc patches. Sorry. holger
On Friday 2014-10-03 08:41, Holger Hans Peter Freyther wrote: >On Thu, Oct 02, 2014 at 11:09:07PM +0200, Jan Engelhardt wrote: > >Good Morning Jan, > >> The link stage fails at some point. libosmogsm.so:lapd-core.c uses >> talloc_free, but does not link to libtalloc.so. Correct this. > >my position has not changed. The --disable-talloc is an option for >Embedded Firmware (e.g. OsmocomBB) that do not want to include talloc. >It is not for people that want to use their distributions talloc. The software does not fully build with --disable-talloc, how could it be of use? (It's possible yes, by sneaking in the talloc symbols in another way, e.g. through a patched libc...) >Using the osmocom/core/talloc.h and linking against another/unknown >version of talloc is scraming for problems (API/ABI issues, etc.) I am aware of this, and wrote this down in p 3/5 (replacing talloc.h with a matching one).
On Fri, Oct 03, 2014 at 11:11:55AM +0200, Jan Engelhardt wrote: Dear Jan, > >my position has not changed. The --disable-talloc is an option for > >Embedded Firmware (e.g. OsmocomBB) that do not want to include talloc. > >It is not for people that want to use their distributions talloc. > > The software does not fully build with --disable-talloc, > how could it be of use? > (It's possible yes, by sneaking in the talloc symbols in > another way, e.g. through a patched libc...) --disable-talloc (and other feature flags) is a special purpose option to reduce the size of the library (e.g. for usage on the OsmocomBB firmware). It is not expected that on a library with reduced functionality the remaining code will build. We had this discussion in 02.2013 and my position has not changed. I agree that we should not have a copy of talloc in our codebase and should rely on the system to provide it. There should not be a osmocom/core/talloc.h and all projects that use talloc should check for the talloc.pc file. Your patch is not moving us in that direction and opens a new can of worms (potential ABI/API incompats, assuming that libtalloc can be found in the library paths passed to the linker, etc.) kind regards holger
On Friday 2014-10-03 12:43, Holger Hans Peter Freyther wrote: >On Fri, Oct 03, 2014 at 11:11:55AM +0200, Jan Engelhardt wrote: > >Dear Jan, > >> >my position has not changed. The --disable-talloc is an option for >> >Embedded Firmware (e.g. OsmocomBB) that do not want to include talloc. >> >It is not for people that want to use their distributions talloc. >> >> The software does not fully build with --disable-talloc, >> how could it be of use? >> (It's possible yes, by sneaking in the talloc symbols in >> another way, e.g. through a patched libc...) > > >--disable-talloc (and other feature flags) is a special purpose option >to reduce the size of the library (e.g. for usage on the OsmocomBB >firmware). >It is not expected that on a library with reduced functionality >the remaining code will build. However, --disable-talloc does not reduce any functionality; it is just a fancy name for --use-system-talloc-in-half-the-package. I am sending a new set soon that completes the system-talloc support, with optionally using the bundled talloc for whoever needs it. Support for both methods has mostly been there anyway already.
diff --git a/src/gsm/Makefile.am b/src/gsm/Makefile.am index 4207959..df72bf7 100644 --- a/src/gsm/Makefile.am +++ b/src/gsm/Makefile.am @@ -23,5 +23,8 @@ libosmogsm_la_SOURCES = a5.c rxlev_stat.c tlv_parser.c comp128.c comp128v23.c \ libosmogsm_la_LDFLAGS = $(LTLDFLAGS_OSMOGSM) -version-info $(LIBVERSION) -no-undefined libosmogsm_la_LIBADD = $(top_builddir)/src/libosmocore.la +if !ENABLE_TALLOC +libosmogsm_la_LIBDADD = -ltalloc +endif EXTRA_DIST = libosmogsm.map