[PATCH 4/5] build: remove unused all_includes and use AM_CPPFLAGS

[Jan Engelhardt]

Preprocessor flags are best placed in AM_CPPFLAGS. Remove use of the
unused all_includes variable, which is never set.
---
Makefile.am | 1 -
src/Makefile.am | 3 ++-
src/codec/Makefile.am | 3 ++-
src/gb/Makefile.am | 3 ++-
src/gsm/Makefile.am | 3 ++-
src/vty/Makefile.am | 3 ++-
tests/Makefile.am | 3 ++-
utils/Makefile.am | 3 ++-
8 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index f2899cc..faf7a83 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,6 +1,5 @@
ACLOCAL_AMFLAGS = -I m4

-AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include
SUBDIRS = include src src/vty src/codec src/gsm src/gb src/ctrl tests utils

pkgconfigdir = $(libdir)/pkgconfig
diff --git a/src/Makefile.am b/src/Makefile.am
index 841f672..29f981d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2,7 +2,8 @@
# Please read Chapter 6 "Library interface versions" of the libtool documentation before making any modification
LIBVERSION=4:0:0

-AM_CFLAGS = -Wall $(all_includes) -I$(top_srcdir)/include -I$(top_builddir)/include
+AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include
+AM_CFLAGS = -Wall

lib_LTLIBRARIES = libosmocore.la

diff --git a/src/codec/Makefile.am b/src/codec/Makefile.am
index f4dec62..b4834e5 100644
--- a/src/codec/Makefile.am
+++ b/src/codec/Makefile.am
@@ -2,7 +2,8 @@
# Please read Chapter 6 "Library interface versions" of the libtool documentation before making any modification
LIBVERSION=0:0:0

-AM_CFLAGS = -Wall $(all_includes) -I$(top_srcdir)/include
+AM_CPPFLAGS = -I$(top_srcdir)/include
+AM_CFLAGS = -Wall

lib_LTLIBRARIES = libosmocodec.la

diff --git a/src/gb/Makefile.am b/src/gb/Makefile.am
index 83af71d..ba85b4f 100644
--- a/src/gb/Makefile.am
+++ b/src/gb/Makefile.am
@@ -2,7 +2,8 @@
# Please read Chapter 6 "Library interface versions" of the libtool documentation before making any modification
LIBVERSION=3:1:0

-AM_CFLAGS = -Wall ${GCC_FVISIBILITY_HIDDEN} -fno-strict-aliasing $(all_includes) -I$(top_srcdir)/include
+AM_CPPFLAGS = -I$(top_srcdir)/include
+AM_CFLAGS = -Wall ${GCC_FVISIBILITY_HIDDEN} -fno-strict-aliasing

# FIXME: this should eventually go into a milenage/Makefile.am
noinst_HEADERS = common_vty.h
diff --git a/src/gsm/Makefile.am b/src/gsm/Makefile.am
index df72bf7..8d94ae1 100644
--- a/src/gsm/Makefile.am
+++ b/src/gsm/Makefile.am
@@ -2,7 +2,8 @@
# Please read Chapter 6 "Library interface versions" of the libtool documentation before making any modification
LIBVERSION=6:0:1

-AM_CFLAGS = -Wall ${GCC_FVISIBILITY_HIDDEN} $(all_includes) -I$(top_srcdir)/include
+AM_CPPFLAGS = -I$(top_srcdir)/include
+AM_CFLAGS = -Wall ${GCC_FVISIBILITY_HIDDEN}

# FIXME: this should eventually go into a milenage/Makefile.am
noinst_HEADERS = milenage/aes.h milenage/aes_i.h milenage/aes_wrap.h \
diff --git a/src/vty/Makefile.am b/src/vty/Makefile.am
index d45659e..52b7ba5 100644
--- a/src/vty/Makefile.am
+++ b/src/vty/Makefile.am
@@ -2,7 +2,8 @@
# Please read Chapter 6 "Library interface versions" of the libtool documentation before making any modification
LIBVERSION=0:1:0

-AM_CFLAGS = -Wall $(all_includes) -I$(top_srcdir)/include
+AM_CPPFLAGS = -I$(top_srcdir)/include
+AM_CFLAGS = -Wall

if ENABLE_VTY
lib_LTLIBRARIES = libosmovty.la
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ea4bf9c..8779c4f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,4 +1,5 @@
-AM_CFLAGS = -Wall $(all_includes) -I$(top_srcdir)/include
+AM_CPPFLAGS = -I$(top_srcdir)/include
+AM_CFLAGS = -Wall

check_PROGRAMS = timer/timer_test sms/sms_test ussd/ussd_test \
smscb/smscb_test bits/bitrev_test a5/a5_test \
diff --git a/utils/Makefile.am b/utils/Makefile.am
index 6a3cd12..51c884a 100644
--- a/utils/Makefile.am
+++ b/utils/Makefile.am
@@ -1,5 +1,6 @@
if ENABLE_UTILITIES
-AM_CFLAGS = -Wall $(all_includes) -I$(top_srcdir)/include
+AM_CPPFLAGS = -I$(top_srcdir)/include
+AM_CFLAGS = -Wall

bin_PROGRAMS = osmo-arfcn osmo-auc-gen

[Jan Engelhardt]

Next-level packages like libosmo-abis strictly depend on
<osmocom/core/talloc.h>, therefore always ship this file. When
distributions are using --disable-talloc to use a system talloc
instead of the shipped copy, they will simply have to overwrite
osmocom/core/talloc.h with the single line #include <talloc.h>.
---
include/Makefile.am | 2 --
1 file changed, 2 deletions(-)

diff --git a/include/Makefile.am b/include/Makefile.am
index c59f9b2..e971e42 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -89,9 +89,7 @@ if ENABLE_PLUGIN
nobase_include_HEADERS += osmocom/core/plugin.h
endif

-if ENABLE_TALLOC
nobase_include_HEADERS += osmocom/core/talloc.h
-endif

if ENABLE_MSGFILE
nobase_include_HEADERS += osmocom/core/msgfile.h

[Jan Engelhardt]

When the osmocore package is configured with --disable-static,
building of kasumi_test fails. This seems quite legit, given
the function _kasumi_kgcore is not exported.
Don't try to workaround the build system. Include the code.

CCLD kasumi/kasumi_test
kasumi_test.o: In function `test_expansion':
~tests/kasumi/kasumi_test.c:25: undefined reference to `_kasumi_key_expand'
kasumi_test.o: In function `main':
~tests/kasumi/kasumi_test.c:56: undefined reference to `_kasumi'
~tests/kasumi/kasumi_test.c:100: undefined reference to `_kasumi_key_expand'
~tests/kasumi/kasumi_test.c:112: undefined reference to `_kasumi_kgcore'
[...]
---
tests/Makefile.am | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8779c4f..3f7db1f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -20,9 +20,8 @@ utils_utils_test_LDADD = $(top_builddir)/src/libosmocore.la
a5_a5_test_SOURCES = a5/a5_test.c
a5_a5_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/gsm/libosmogsm.la

-kasumi_kasumi_test_SOURCES = kasumi/kasumi_test.c
+kasumi_kasumi_test_SOURCES = kasumi/kasumi_test.c ../src/gsm/kasumi.c
kasumi_kasumi_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/gsm/libosmogsm.la
-kasumi_kasumi_test_LDFLAGS = -static

comp128_comp128_test_SOURCES = comp128/comp128_test.c
comp128_comp128_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/gsm/libosmogsm.la

[Jan Engelhardt]

CC osmo-auc-gen.o
osmo-auc-gen.c: In function 'main':
osmo-auc-gen.c:216:3: warning: implicit declaration of function
'time' [-Wimplicit-function-declaration]
---
utils/osmo-auc-gen.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/utils/osmo-auc-gen.c b/utils/osmo-auc-gen.c
index fda045a..54128ff 100644
--- a/utils/osmo-auc-gen.c
+++ b/utils/osmo-auc-gen.c
@@ -25,6 +25,7 @@
#include <stdio.h>
#include <errno.h>
#include <string.h>
+#include <time.h>
#include <getopt.h>
#include <unistd.h>
#include <inttypes.h>

[Jan Engelhardt]

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(+)

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

[Holger Hans Peter Freyther]

On Thu, Oct 02, 2014 at 11:09:07PM +0200, Jan Engelhardt wrote:

Good Morning Jan,

Post by Jan Engelhardt
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]

Post by Holger Hans Peter Freyther
Good Morning Jan,

Post by Jan Engelhardt
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...)

Post by Holger Hans Peter Freyther
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 Hans Peter Freyther]

On Fri, Oct 03, 2014 at 11:11:55AM +0200, Jan Engelhardt wrote:

Dear Jan,

Post by Jan Engelhardt

Post by Holger Hans Peter Freyther
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]

Post by Holger Hans Peter Freyther
Dear Jan,

Post by Jan Engelhardt

Post by Holger Hans Peter Freyther
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.