[2/5] build: resolve link failure in libosmogsm when --disable-talloc is used
diff mbox

Message ID 1412284150-11771-3-git-send-email-jengelh@inai.de
State Rejected
Headers show

Commit Message

Jan Engelhardt Oct. 2, 2014, 9:09 p.m. UTC
The link stage fails at some point. libosmogsm.so:lapd-core.c uses
talloc_free, but does not link to libtalloc.so. Correct this.

	   CCLD     osmo-arfcn
	../src/gsm/.libs/libosmogsm.so: undefined reference to `talloc_free'
	collect2: error: ld returned 1 exit status
	make[2]: *** [osmo-arfcn] Error 1
---
 src/gsm/Makefile.am | 3 +++
 1 file changed, 3 insertions(+)

Comments

Holger Freyther Oct. 3, 2014, 6:41 a.m. UTC | #1
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
Jan Engelhardt Oct. 3, 2014, 9:11 a.m. UTC | #2
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).
Holger Freyther Oct. 3, 2014, 10:43 a.m. UTC | #3
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
Jan Engelhardt Oct. 3, 2014, 10:53 a.m. UTC | #4
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.

Patch
diff mbox

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