"--disable-talloc" had the effect of a "--enable-system-talloc". So the first patch fixes up all the link failures by adding -ltalloc where needed, _for the case that one decides to use disable-talloc_.
Second patch will use pkg-config to determine the parameters for a system talloc. These will be used unconditionally, because the variables TALLOC_CFLAGS/TALLOC_LIBS will be empty when using the bundled talloc.
=== The following changes since commit c60de4f35f3f73f3d79b4ff61d0a20b92f818702:
build: fix linker error with kasumi_test (2014-10-03 08:48:31 +0200)
are available in the git repository at:
git://git.inai.de/libosmocore HEAD
for you to fetch changes up to d6c1607a724a07416c7d39f9b43f7af7349de666:
build: default to system-provided talloc (2014-10-03 12:43:25 +0200)
---------------------------------------------------------------- Jan Engelhardt (3): build: resolve link failure in libosmogsm when --disable-talloc is used build: allow using a system-provided talloc properly build: default to system-provided talloc
.gitignore | 1 + configure.ac | 26 +++++++++++++++++++------ include/Makefile.am | 5 +++-- include/osmocom/core/talloc.h.in | 6 ++++++ include/osmocom/core/{talloc.h => talloc_int.h} | 0 src/Makefile.am | 7 ++++--- src/gsm/Makefile.am | 5 +++-- tests/Makefile.am | 12 ++++++------ tests/msgfile/msgfile_test.c | 1 + 9 files changed, 44 insertions(+), 19 deletions(-) create mode 100644 include/osmocom/core/talloc.h.in rename include/osmocom/core/{talloc.h => talloc_int.h} (100%)
libosmogsm.so:lapd-core.c uses talloc_free unconditionally (like libosmocore), but in the absence of the builtin talloc, does not link to libtalloc.so (like libosmocore would). This causes linker failures when programs using libosmogsm are being built. Correct this issue.
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 828aa50..b0cdea2 100644 --- a/src/gsm/Makefile.am +++ b/src/gsm/Makefile.am @@ -24,5 +24,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_LIBADD += -ltalloc +endif
EXTRA_DIST = libosmogsm.map
On Fri, Oct 03, 2014 at 12:57:02PM +0200, Jan Engelhardt wrote:
Hi!
+if !ENABLE_TALLOC +libosmogsm_la_LIBADD += -ltalloc +endif
please see my other mail. You assume that a libtalloc exists on the system and that it is found in the library search paths. This is not an improvement, it is a change that breaks in different ways for different people.
As in 02.2013 and as of today. I really appreciate your work but I can't take this patch.
holger
On Friday 2014-10-03 13:04, Holger Hans Peter Freyther wrote:
On Fri, Oct 03, 2014 at 12:57:02PM +0200, Jan Engelhardt wrote:
+if !ENABLE_TALLOC +libosmogsm_la_LIBADD += -ltalloc +endif
You assume that a libtalloc exists on the system and that it is found in the library search paths. This is not an improvement, it is a change that breaks in different ways for different people.
It is the same logic that was used for libosmocore_la_LIBADD. It goes away with patch 2 anyway, so it should not hurt. Squash the two if need be.
When --disable-talloc was given to configure, it would already link libosmocore.so to libtalloc. Make it use pkg-config to discover talloc's flags and location, and when the system talloc is used, make sure to use its API header file too. --- .gitignore | 1 + configure.ac | 16 +++- include/Makefile.am | 3 +- include/osmocom/core/talloc.h | 189 -------------------------------------- include/osmocom/core/talloc.h.in | 6 ++ include/osmocom/core/talloc_int.h | 189 ++++++++++++++++++++++++++++++++++++++ src/Makefile.am | 5 +- src/gsm/Makefile.am | 8 +- tests/Makefile.am | 12 +-- tests/msgfile/msgfile_test.c | 1 + 10 files changed, 226 insertions(+), 204 deletions(-) delete mode 100644 include/osmocom/core/talloc.h create mode 100644 include/osmocom/core/talloc.h.in create mode 100644 include/osmocom/core/talloc_int.h
diff --git a/.gitignore b/.gitignore index 1299028..30707a6 100644 --- a/.gitignore +++ b/.gitignore @@ -92,6 +92,7 @@ doc/html.tar src/crc*gen.c include/osmocom/core/crc*gen.h include/osmocom/core/bit*gen.h +/include/osmocom/core/talloc.h
# vi files *.sw? diff --git a/configure.ac b/configure.ac index deaa8bf..f0d1206 100644 --- a/configure.ac +++ b/configure.ac @@ -85,10 +85,23 @@ AC_CONFIG_HEADER(config.h) AC_ARG_ENABLE(talloc, [AS_HELP_STRING( [--disable-talloc], - [Disable building talloc memory allocator] + [Disable using bundled talloc memory allocator] )], [enable_talloc=$enableval], [enable_talloc="yes"]) AM_CONDITIONAL(ENABLE_TALLOC, [test x"$enable_talloc" = x"yes"]) +AS_IF([test x"$enable_talloc" = xno], + [ + PKG_CHECK_MODULES([TALLOC], [talloc]) + OSMOCORE_INTERNAL_TALLOC=0 + ], + [ + TALLOC_CFLAGS="" + TALLOC_LIBS="" + OSMOCORE_INTERNAL_TALLOC=1 + AC_SUBST([TALLOC_CFLAGS]) + AC_SUBST([TALLOC_LIBS]) + ]) +AC_SUBST([OSMOCORE_INTERNAL_TALLOC])
AC_ARG_ENABLE(plugin, [AS_HELP_STRING( @@ -188,6 +201,7 @@ AC_OUTPUT( libosmogb.pc libosmoctrl.pc include/Makefile + include/osmocom/core/talloc.h src/Makefile src/vty/Makefile src/codec/Makefile diff --git a/include/Makefile.am b/include/Makefile.am index c59f9b2..4d59a72 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -89,8 +89,9 @@ if ENABLE_PLUGIN nobase_include_HEADERS += osmocom/core/plugin.h endif
+nobase_nodist_include_HEADERS = osmocom/core/talloc.h if ENABLE_TALLOC -nobase_include_HEADERS += osmocom/core/talloc.h +nobase_include_HEADERS += osmocom/core/talloc_int.h endif
if ENABLE_MSGFILE diff --git a/include/osmocom/core/talloc.h b/include/osmocom/core/talloc.h deleted file mode 100644 index dde102b..0000000 --- a/include/osmocom/core/talloc.h +++ /dev/null @@ -1,189 +0,0 @@ -#pragma once -/* - Unix SMB/CIFS implementation. - Samba temporary memory allocation functions - - Copyright (C) Andrew Tridgell 2004-2005 - Copyright (C) Stefan Metzmacher 2006 - - ** NOTE! The following LGPL license applies to the talloc - ** library. This does NOT imply that all of Samba is released - ** under the LGPL - - This library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 3 of the License, or (at your option) any later version. - - This library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with this library; if not, see http://www.gnu.org/licenses/. -*/ - -#include <stdlib.h> -#include <stdio.h> -#include <stdarg.h> - -#define HAVE_VA_COPY - -/* this is only needed for compatibility with the old talloc */ -typedef void TALLOC_CTX; - -/* - this uses a little trick to allow __LINE__ to be stringified -*/ -#ifndef __location__ -#define __TALLOC_STRING_LINE1__(s) #s -#define __TALLOC_STRING_LINE2__(s) __TALLOC_STRING_LINE1__(s) -#define __TALLOC_STRING_LINE3__ __TALLOC_STRING_LINE2__(__LINE__) -#define __location__ __FILE__ ":" __TALLOC_STRING_LINE3__ -#endif - -#ifndef TALLOC_DEPRECATED -#define TALLOC_DEPRECATED 0 -#endif - -#ifndef PRINTF_ATTRIBUTE -#if (__GNUC__ >= 3) -/** Use gcc attribute to check printf fns. a1 is the 1-based index of - * the parameter containing the format, and a2 the index of the first - * argument. Note that some gcc 2.x versions don't handle this - * properly **/ -#define PRINTF_ATTRIBUTE(a1, a2) __attribute__ ((format (__printf__, a1, a2))) -#else -#define PRINTF_ATTRIBUTE(a1, a2) -#endif -#endif - -/* try to make talloc_set_destructor() and talloc_steal() type safe, - if we have a recent gcc */ -#if (__GNUC__ >= 3) -#define _TALLOC_TYPEOF(ptr) __typeof__(ptr) -#define talloc_set_destructor(ptr, function) \ - do { \ - int (*_talloc_destructor_fn)(_TALLOC_TYPEOF(ptr)) = (function); \ - _talloc_set_destructor((ptr), (int (*)(void *))_talloc_destructor_fn); \ - } while(0) -/* this extremely strange macro is to avoid some braindamaged warning - stupidity in gcc 4.1.x */ -#define talloc_steal(ctx, ptr) ({ _TALLOC_TYPEOF(ptr) __talloc_steal_ret = (_TALLOC_TYPEOF(ptr))_talloc_steal((ctx),(ptr)); __talloc_steal_ret; }) -#else -#define talloc_set_destructor(ptr, function) \ - _talloc_set_destructor((ptr), (int (*)(void *))(function)) -#define _TALLOC_TYPEOF(ptr) void * -#define talloc_steal(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_steal((ctx),(ptr)) -#endif - -#define talloc_reference(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_reference((ctx),(ptr)) -#define talloc_move(ctx, ptr) (_TALLOC_TYPEOF(*(ptr)))_talloc_move((ctx),(void *)(ptr)) - -/* useful macros for creating type checked pointers */ -#define talloc(ctx, type) (type *)talloc_named_const(ctx, sizeof(type), #type) -#define talloc_size(ctx, size) talloc_named_const(ctx, size, __location__) -#define talloc_ptrtype(ctx, ptr) (_TALLOC_TYPEOF(ptr))talloc_size(ctx, sizeof(*(ptr))) - -#define talloc_new(ctx) talloc_named_const(ctx, 0, "talloc_new: " __location__) - -#define talloc_zero(ctx, type) (type *)_talloc_zero(ctx, sizeof(type), #type) -#define talloc_zero_size(ctx, size) _talloc_zero(ctx, size, __location__) - -#define talloc_zero_array(ctx, type, count) (type *)_talloc_zero_array(ctx, sizeof(type), count, #type) -#define talloc_array(ctx, type, count) (type *)_talloc_array(ctx, sizeof(type), count, #type) -#define talloc_array_size(ctx, size, count) _talloc_array(ctx, size, count, __location__) -#define talloc_array_ptrtype(ctx, ptr, count) (_TALLOC_TYPEOF(ptr))talloc_array_size(ctx, sizeof(*(ptr)), count) -#define talloc_array_length(ctx) (talloc_get_size(ctx)/sizeof(*ctx)) - -#define talloc_realloc(ctx, p, type, count) (type *)_talloc_realloc_array(ctx, p, sizeof(type), count, #type) -#define talloc_realloc_size(ctx, ptr, size) _talloc_realloc(ctx, ptr, size, __location__) - -#define talloc_memdup(t, p, size) _talloc_memdup(t, p, size, __location__) - -#define talloc_set_type(ptr, type) talloc_set_name_const(ptr, #type) -#define talloc_get_type(ptr, type) (type *)talloc_check_name(ptr, #type) -#define talloc_get_type_abort(ptr, type) (type *)_talloc_get_type_abort(ptr, #type, __location__) - -#define talloc_find_parent_bytype(ptr, type) (type *)talloc_find_parent_byname(ptr, #type) - -#if TALLOC_DEPRECATED -#define talloc_zero_p(ctx, type) talloc_zero(ctx, type) -#define talloc_p(ctx, type) talloc(ctx, type) -#define talloc_array_p(ctx, type, count) talloc_array(ctx, type, count) -#define talloc_realloc_p(ctx, p, type, count) talloc_realloc(ctx, p, type, count) -#define talloc_destroy(ctx) talloc_free(ctx) -#define talloc_append_string(c, s, a) (s?talloc_strdup_append(s,a):talloc_strdup(c, a)) -#endif - -#define TALLOC_FREE(ctx) do { talloc_free(ctx); ctx=NULL; } while(0) - -/* The following definitions come from talloc.c */ -void *_talloc(const void *context, size_t size); -void *talloc_pool(const void *context, size_t size); -void _talloc_set_destructor(const void *ptr, int (*_destructor)(void *)); -int talloc_increase_ref_count(const void *ptr); -size_t talloc_reference_count(const void *ptr); -void *_talloc_reference(const void *context, const void *ptr); -int talloc_unlink(const void *context, void *ptr); -const char *talloc_set_name(const void *ptr, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3); -void talloc_set_name_const(const void *ptr, const char *name); -void *talloc_named(const void *context, size_t size, - const char *fmt, ...) PRINTF_ATTRIBUTE(3,4); -void *talloc_named_const(const void *context, size_t size, const char *name); -const char *talloc_get_name(const void *ptr); -void *talloc_check_name(const void *ptr, const char *name); -void *_talloc_get_type_abort(const void *ptr, const char *name, const char *location); -void *talloc_parent(const void *ptr); -const char *talloc_parent_name(const void *ptr); -void *talloc_init(const char *fmt, ...) PRINTF_ATTRIBUTE(1,2); -int talloc_free(void *ptr); -void talloc_free_children(void *ptr); -void *_talloc_realloc(const void *context, void *ptr, size_t size, const char *name); -void *_talloc_steal(const void *new_ctx, const void *ptr); -void *_talloc_move(const void *new_ctx, const void *pptr); -size_t talloc_total_size(const void *ptr); -size_t talloc_total_blocks(const void *ptr); -void talloc_report_depth_cb(const void *ptr, int depth, int max_depth, - void (*callback)(const void *ptr, - int depth, int max_depth, - int is_ref, - void *private_data), - void *private_data); -void talloc_report_depth_file(const void *ptr, int depth, int max_depth, FILE *f); -void talloc_report_full(const void *ptr, FILE *f); -void talloc_report(const void *ptr, FILE *f); -void talloc_enable_null_tracking(void); -void talloc_disable_null_tracking(void); -void talloc_enable_leak_report(void); -void talloc_enable_leak_report_full(void); -void *_talloc_zero(const void *ctx, size_t size, const char *name); -void *_talloc_memdup(const void *t, const void *p, size_t size, const char *name); -void *_talloc_array(const void *ctx, size_t el_size, unsigned count, const char *name); -void *_talloc_zero_array(const void *ctx, size_t el_size, unsigned count, const char *name); -void *_talloc_realloc_array(const void *ctx, void *ptr, size_t el_size, unsigned count, const char *name); -void *talloc_realloc_fn(const void *context, void *ptr, size_t size); -void *talloc_autofree_context(void); -size_t talloc_get_size(const void *ctx); -void *talloc_find_parent_byname(const void *ctx, const char *name); -void talloc_show_parents(const void *context, FILE *file); -int talloc_is_parent(const void *context, const void *ptr); - -char *talloc_strdup(const void *t, const char *p); -char *talloc_strdup_append(char *s, const char *a); -char *talloc_strdup_append_buffer(char *s, const char *a); - -char *talloc_strndup(const void *t, const char *p, size_t n); -char *talloc_strndup_append(char *s, const char *a, size_t n); -char *talloc_strndup_append_buffer(char *s, const char *a, size_t n); - -char *talloc_vasprintf(const void *t, const char *fmt, va_list ap) PRINTF_ATTRIBUTE(2,0); -char *talloc_vasprintf_append(char *s, const char *fmt, va_list ap) PRINTF_ATTRIBUTE(2,0); -char *talloc_vasprintf_append_buffer(char *s, const char *fmt, va_list ap) PRINTF_ATTRIBUTE(2,0); - -char *talloc_asprintf(const void *t, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3); -char *talloc_asprintf_append(char *s, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3); -char *talloc_asprintf_append_buffer(char *s, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3); - -void talloc_set_abort_fn(void (*abort_fn)(const char *reason)); diff --git a/include/osmocom/core/talloc.h.in b/include/osmocom/core/talloc.h.in new file mode 100644 index 0000000..edbbb63 --- /dev/null +++ b/include/osmocom/core/talloc.h.in @@ -0,0 +1,6 @@ +#pragma once +#if @OSMOCORE_INTERNAL_TALLOC@ +#include <osmocom/core/talloc_int.h> +#else +#include <talloc.h> +#endif diff --git a/include/osmocom/core/talloc_int.h b/include/osmocom/core/talloc_int.h new file mode 100644 index 0000000..dde102b --- /dev/null +++ b/include/osmocom/core/talloc_int.h @@ -0,0 +1,189 @@ +#pragma once +/* + Unix SMB/CIFS implementation. + Samba temporary memory allocation functions + + Copyright (C) Andrew Tridgell 2004-2005 + Copyright (C) Stefan Metzmacher 2006 + + ** NOTE! The following LGPL license applies to the talloc + ** library. This does NOT imply that all of Samba is released + ** under the LGPL + + This library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 3 of the License, or (at your option) any later version. + + This library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with this library; if not, see http://www.gnu.org/licenses/. +*/ + +#include <stdlib.h> +#include <stdio.h> +#include <stdarg.h> + +#define HAVE_VA_COPY + +/* this is only needed for compatibility with the old talloc */ +typedef void TALLOC_CTX; + +/* + this uses a little trick to allow __LINE__ to be stringified +*/ +#ifndef __location__ +#define __TALLOC_STRING_LINE1__(s) #s +#define __TALLOC_STRING_LINE2__(s) __TALLOC_STRING_LINE1__(s) +#define __TALLOC_STRING_LINE3__ __TALLOC_STRING_LINE2__(__LINE__) +#define __location__ __FILE__ ":" __TALLOC_STRING_LINE3__ +#endif + +#ifndef TALLOC_DEPRECATED +#define TALLOC_DEPRECATED 0 +#endif + +#ifndef PRINTF_ATTRIBUTE +#if (__GNUC__ >= 3) +/** Use gcc attribute to check printf fns. a1 is the 1-based index of + * the parameter containing the format, and a2 the index of the first + * argument. Note that some gcc 2.x versions don't handle this + * properly **/ +#define PRINTF_ATTRIBUTE(a1, a2) __attribute__ ((format (__printf__, a1, a2))) +#else +#define PRINTF_ATTRIBUTE(a1, a2) +#endif +#endif + +/* try to make talloc_set_destructor() and talloc_steal() type safe, + if we have a recent gcc */ +#if (__GNUC__ >= 3) +#define _TALLOC_TYPEOF(ptr) __typeof__(ptr) +#define talloc_set_destructor(ptr, function) \ + do { \ + int (*_talloc_destructor_fn)(_TALLOC_TYPEOF(ptr)) = (function); \ + _talloc_set_destructor((ptr), (int (*)(void *))_talloc_destructor_fn); \ + } while(0) +/* this extremely strange macro is to avoid some braindamaged warning + stupidity in gcc 4.1.x */ +#define talloc_steal(ctx, ptr) ({ _TALLOC_TYPEOF(ptr) __talloc_steal_ret = (_TALLOC_TYPEOF(ptr))_talloc_steal((ctx),(ptr)); __talloc_steal_ret; }) +#else +#define talloc_set_destructor(ptr, function) \ + _talloc_set_destructor((ptr), (int (*)(void *))(function)) +#define _TALLOC_TYPEOF(ptr) void * +#define talloc_steal(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_steal((ctx),(ptr)) +#endif + +#define talloc_reference(ctx, ptr) (_TALLOC_TYPEOF(ptr))_talloc_reference((ctx),(ptr)) +#define talloc_move(ctx, ptr) (_TALLOC_TYPEOF(*(ptr)))_talloc_move((ctx),(void *)(ptr)) + +/* useful macros for creating type checked pointers */ +#define talloc(ctx, type) (type *)talloc_named_const(ctx, sizeof(type), #type) +#define talloc_size(ctx, size) talloc_named_const(ctx, size, __location__) +#define talloc_ptrtype(ctx, ptr) (_TALLOC_TYPEOF(ptr))talloc_size(ctx, sizeof(*(ptr))) + +#define talloc_new(ctx) talloc_named_const(ctx, 0, "talloc_new: " __location__) + +#define talloc_zero(ctx, type) (type *)_talloc_zero(ctx, sizeof(type), #type) +#define talloc_zero_size(ctx, size) _talloc_zero(ctx, size, __location__) + +#define talloc_zero_array(ctx, type, count) (type *)_talloc_zero_array(ctx, sizeof(type), count, #type) +#define talloc_array(ctx, type, count) (type *)_talloc_array(ctx, sizeof(type), count, #type) +#define talloc_array_size(ctx, size, count) _talloc_array(ctx, size, count, __location__) +#define talloc_array_ptrtype(ctx, ptr, count) (_TALLOC_TYPEOF(ptr))talloc_array_size(ctx, sizeof(*(ptr)), count) +#define talloc_array_length(ctx) (talloc_get_size(ctx)/sizeof(*ctx)) + +#define talloc_realloc(ctx, p, type, count) (type *)_talloc_realloc_array(ctx, p, sizeof(type), count, #type) +#define talloc_realloc_size(ctx, ptr, size) _talloc_realloc(ctx, ptr, size, __location__) + +#define talloc_memdup(t, p, size) _talloc_memdup(t, p, size, __location__) + +#define talloc_set_type(ptr, type) talloc_set_name_const(ptr, #type) +#define talloc_get_type(ptr, type) (type *)talloc_check_name(ptr, #type) +#define talloc_get_type_abort(ptr, type) (type *)_talloc_get_type_abort(ptr, #type, __location__) + +#define talloc_find_parent_bytype(ptr, type) (type *)talloc_find_parent_byname(ptr, #type) + +#if TALLOC_DEPRECATED +#define talloc_zero_p(ctx, type) talloc_zero(ctx, type) +#define talloc_p(ctx, type) talloc(ctx, type) +#define talloc_array_p(ctx, type, count) talloc_array(ctx, type, count) +#define talloc_realloc_p(ctx, p, type, count) talloc_realloc(ctx, p, type, count) +#define talloc_destroy(ctx) talloc_free(ctx) +#define talloc_append_string(c, s, a) (s?talloc_strdup_append(s,a):talloc_strdup(c, a)) +#endif + +#define TALLOC_FREE(ctx) do { talloc_free(ctx); ctx=NULL; } while(0) + +/* The following definitions come from talloc.c */ +void *_talloc(const void *context, size_t size); +void *talloc_pool(const void *context, size_t size); +void _talloc_set_destructor(const void *ptr, int (*_destructor)(void *)); +int talloc_increase_ref_count(const void *ptr); +size_t talloc_reference_count(const void *ptr); +void *_talloc_reference(const void *context, const void *ptr); +int talloc_unlink(const void *context, void *ptr); +const char *talloc_set_name(const void *ptr, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3); +void talloc_set_name_const(const void *ptr, const char *name); +void *talloc_named(const void *context, size_t size, + const char *fmt, ...) PRINTF_ATTRIBUTE(3,4); +void *talloc_named_const(const void *context, size_t size, const char *name); +const char *talloc_get_name(const void *ptr); +void *talloc_check_name(const void *ptr, const char *name); +void *_talloc_get_type_abort(const void *ptr, const char *name, const char *location); +void *talloc_parent(const void *ptr); +const char *talloc_parent_name(const void *ptr); +void *talloc_init(const char *fmt, ...) PRINTF_ATTRIBUTE(1,2); +int talloc_free(void *ptr); +void talloc_free_children(void *ptr); +void *_talloc_realloc(const void *context, void *ptr, size_t size, const char *name); +void *_talloc_steal(const void *new_ctx, const void *ptr); +void *_talloc_move(const void *new_ctx, const void *pptr); +size_t talloc_total_size(const void *ptr); +size_t talloc_total_blocks(const void *ptr); +void talloc_report_depth_cb(const void *ptr, int depth, int max_depth, + void (*callback)(const void *ptr, + int depth, int max_depth, + int is_ref, + void *private_data), + void *private_data); +void talloc_report_depth_file(const void *ptr, int depth, int max_depth, FILE *f); +void talloc_report_full(const void *ptr, FILE *f); +void talloc_report(const void *ptr, FILE *f); +void talloc_enable_null_tracking(void); +void talloc_disable_null_tracking(void); +void talloc_enable_leak_report(void); +void talloc_enable_leak_report_full(void); +void *_talloc_zero(const void *ctx, size_t size, const char *name); +void *_talloc_memdup(const void *t, const void *p, size_t size, const char *name); +void *_talloc_array(const void *ctx, size_t el_size, unsigned count, const char *name); +void *_talloc_zero_array(const void *ctx, size_t el_size, unsigned count, const char *name); +void *_talloc_realloc_array(const void *ctx, void *ptr, size_t el_size, unsigned count, const char *name); +void *talloc_realloc_fn(const void *context, void *ptr, size_t size); +void *talloc_autofree_context(void); +size_t talloc_get_size(const void *ctx); +void *talloc_find_parent_byname(const void *ctx, const char *name); +void talloc_show_parents(const void *context, FILE *file); +int talloc_is_parent(const void *context, const void *ptr); + +char *talloc_strdup(const void *t, const char *p); +char *talloc_strdup_append(char *s, const char *a); +char *talloc_strdup_append_buffer(char *s, const char *a); + +char *talloc_strndup(const void *t, const char *p, size_t n); +char *talloc_strndup_append(char *s, const char *a, size_t n); +char *talloc_strndup_append_buffer(char *s, const char *a, size_t n); + +char *talloc_vasprintf(const void *t, const char *fmt, va_list ap) PRINTF_ATTRIBUTE(2,0); +char *talloc_vasprintf_append(char *s, const char *fmt, va_list ap) PRINTF_ATTRIBUTE(2,0); +char *talloc_vasprintf_append_buffer(char *s, const char *fmt, va_list ap) PRINTF_ATTRIBUTE(2,0); + +char *talloc_asprintf(const void *t, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3); +char *talloc_asprintf_append(char *s, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3); +char *talloc_asprintf_append_buffer(char *s, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3); + +void talloc_set_abort_fn(void (*abort_fn)(const char *reason)); diff --git a/src/Makefile.am b/src/Makefile.am index 65494a5..3924330 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=5:0:0
-AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include +AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include \ + $(TALLOC_CFLAGS) AM_CFLAGS = -Wall
lib_LTLIBRARIES = libosmocore.la @@ -29,7 +30,7 @@ endif if ENABLE_TALLOC libosmocore_la_SOURCES += talloc.c else -libosmocore_la_LIBADD += -ltalloc +libosmocore_la_LIBADD += $(TALLOC_LIBS) endif
if ENABLE_MSGFILE diff --git a/src/gsm/Makefile.am b/src/gsm/Makefile.am index b0cdea2..40a1821 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_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include +AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include \ + $(TALLOC_CFLAGS) AM_CFLAGS = -Wall ${GCC_FVISIBILITY_HIDDEN}
# FIXME: this should eventually go into a milenage/Makefile.am @@ -23,9 +24,6 @@ libosmogsm_la_SOURCES = a5.c rxlev_stat.c tlv_parser.c comp128.c comp128v23.c \ milenage/milenage.c gan.c ipa.c
libosmogsm_la_LDFLAGS = $(LTLDFLAGS_OSMOGSM) -version-info $(LIBVERSION) -no-undefined -libosmogsm_la_LIBADD = $(top_builddir)/src/libosmocore.la -if !ENABLE_TALLOC -libosmogsm_la_LIBADD += -ltalloc -endif +libosmogsm_la_LIBADD = $(top_builddir)/src/libosmocore.la $(TALLOC_LIBS)
EXTRA_DIST = libosmogsm.map diff --git a/tests/Makefile.am b/tests/Makefile.am index 3f7db1f..9f034c0 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,4 +1,4 @@ -AM_CPPFLAGS = -I$(top_srcdir)/include +AM_CPPFLAGS = -I$(top_srcdir)/include $(TALLOC_CFLAGS) AM_CFLAGS = -Wall
check_PROGRAMS = timer/timer_test sms/sms_test ussd/ussd_test \ @@ -45,7 +45,7 @@ lapd_lapd_test_SOURCES = lapd/lapd_test.c lapd_lapd_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/gsm/libosmogsm.la
msgfile_msgfile_test_SOURCES = msgfile/msgfile_test.c -msgfile_msgfile_test_LDADD = $(top_builddir)/src/libosmocore.la +msgfile_msgfile_test_LDADD = $(top_builddir)/src/libosmocore.la $(TALLOC_LIBS)
smscb_smscb_test_SOURCES = smscb/smscb_test.c smscb_smscb_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/gsm/libosmogsm.la @@ -54,13 +54,13 @@ sms_sms_test_SOURCES = sms/sms_test.c sms_sms_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/gsm/libosmogsm.la
timer_timer_test_SOURCES = timer/timer_test.c -timer_timer_test_LDADD = $(top_builddir)/src/libosmocore.la +timer_timer_test_LDADD = $(top_builddir)/src/libosmocore.la $(TALLOC_LIBS)
ussd_ussd_test_SOURCES = ussd/ussd_test.c ussd_ussd_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/gsm/libosmogsm.la
gb_bssgp_fc_test_SOURCES = gb/bssgp_fc_test.c -gb_bssgp_fc_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/gb/libosmogb.la +gb_bssgp_fc_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/gb/libosmogb.la $(TALLOC_LIBS)
gb_gprs_ns_test_SOURCES = gb/gprs_ns_test.c gb_gprs_ns_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/gb/libosmogb.la $(LIBRARY_DL) @@ -75,10 +75,10 @@ loggingrb_loggingrb_test_SOURCES = logging/logging_test.c loggingrb_loggingrb_test_LDADD = $(top_builddir)/src/libosmocore.la $(top_builddir)/src/vty/libosmovty.la
strrb_strrb_test_SOURCES = strrb/strrb_test.c -strrb_strrb_test_LDADD = $(top_builddir)/src/libosmocore.la +strrb_strrb_test_LDADD = $(top_builddir)/src/libosmocore.la $(TALLOC_LIBS)
vty_vty_test_SOURCES = vty/vty_test.c -vty_vty_test_LDADD = $(top_builddir)/src/vty/libosmovty.la $(top_builddir)/src/libosmocore.la +vty_vty_test_LDADD = $(top_builddir)/src/vty/libosmovty.la $(top_builddir)/src/libosmocore.la $(TALLOC_LIBS)
# The `:;' works around a Bash 3.2 bug when the output is not writeable. diff --git a/tests/msgfile/msgfile_test.c b/tests/msgfile/msgfile_test.c index a919694..2684b6a 100644 --- a/tests/msgfile/msgfile_test.c +++ b/tests/msgfile/msgfile_test.c @@ -20,6 +20,7 @@ */
#include <osmocom/core/msgfile.h> +#include <osmocom/core/talloc.h>
#include <stdio.h>
On Fri, Oct 03, 2014 at 12:57:03PM +0200, Jan Engelhardt wrote:
When --disable-talloc was given to configure, it would already link libosmocore.so to libtalloc. Make it use pkg-config to discover talloc's flags and location, and when the system talloc is used, make sure to use its API header file too.
include/osmocom/core/talloc.h.in | 6 ++
Have you considered patching libosmocore.pc.in as well to list talloc as "required" there? This way users of libosmocore do not need to check for talloc themselves?
$ pkg-config --cflags --libs libosmocore -I/home/ich/install/openbsc/include/ -L/home/ich/install/openbsc/lib -losmocore -ltalloc
AC_ARG_ENABLE(talloc, [AS_HELP_STRING( [--disable-talloc],
[Disable building talloc memory allocator]
[Disable using bundled talloc memory allocator]AM_CONDITIONAL(ENABLE_TALLOC, [test x"$enable_talloc" = x"yes"]) +AS_IF([test x"$enable_talloc" = xno],
I think the wording does not reflect the two different usecases we have here.
* Firmware builds with no talloc * People wanting to use
+#pragma once +#if @OSMOCORE_INTERNAL_TALLOC@ +#include <osmocom/core/talloc_int.h> +#else +#include <talloc.h> +#endif
Let's hope no-one puts -Iinclude/osmocom/core into the compile line or we might end up with recursion. :)
diff --git a/include/osmocom/core/talloc_int.h b/include/osmocom/core/talloc_int.h new file mode 100644 index 0000000..dde102b --- /dev/null +++ b/include/osmocom/core/talloc_int.h
Do you know the '-M' option for send-email/format-patch? It helps with reviewing simple renames.
diff --git a/tests/msgfile/msgfile_test.c b/tests/msgfile/msgfile_test.c index a919694..2684b6a 100644 --- a/tests/msgfile/msgfile_test.c +++ b/tests/msgfile/msgfile_test.c @@ -20,6 +20,7 @@ */
#include <osmocom/core/msgfile.h> +#include <osmocom/core/talloc.h>
Is this hunk needed?
If you address the pkg-config issue I am happy to inclue your patch. In terms of options.. maybe we can just remove --disable-talloc instead of having sort of a tri-state?
thanks a lot holger
On Friday 2014-10-03 13:19, Holger Hans Peter Freyther wrote:
On Fri, Oct 03, 2014 at 12:57:03PM +0200, Jan Engelhardt wrote:
When --disable-talloc was given to configure, it would already link libosmocore.so to libtalloc. Make it use pkg-config to discover talloc's flags and location, and when the system talloc is used, make sure to use its API header file too.
include/osmocom/core/talloc.h.in | 6 ++
Have you considered patching libosmocore.pc.in as well to list talloc as "required" there? This way users of libosmocore do not need to check for talloc themselves?
I can do that.
AC_ARG_ENABLE(talloc, [AS_HELP_STRING( [--disable-talloc],
[Disable building talloc memory allocator]
[Disable using bundled talloc memory allocator]AM_CONDITIONAL(ENABLE_TALLOC, [test x"$enable_talloc" = x"yes"]) +AS_IF([test x"$enable_talloc" = xno],
I think the wording does not reflect the two different usecases we have here.
- Firmware builds with no talloc
That may be the use case, but it is not what was implemented. If you build 0.7.0 [336915aa0a] with --disable-talloc, `ldd src/.libs/libosmocore.so` will show libtalloc.so.2. Which kind of goes against this use case. Therefore, "disable-talloc" really just meant "disable-system-talloc".
diff --git a/include/osmocom/core/talloc_int.h b/include/osmocom/core/talloc_int.h new file mode 100644 index 0000000..dde102b --- /dev/null +++ b/include/osmocom/core/talloc_int.h
Do you know the '-M' option for send-email/format-patch? It helps with reviewing simple renames.
Been there, tried that. And when I used it, comes someone who wants a non-M submission because they are using some traditional utility or something. Not here, but I remember it occurring on netfilter-devel.
diff --git a/tests/msgfile/msgfile_test.c b/tests/msgfile/msgfile_test.c index a919694..2684b6a 100644 --- a/tests/msgfile/msgfile_test.c +++ b/tests/msgfile/msgfile_test.c @@ -20,6 +20,7 @@ */
#include <osmocom/core/msgfile.h> +#include <osmocom/core/talloc.h>
Is this hunk needed?
Yes, because system talloc has a
#define talloc_free(ctx) _talloc_free(ctx, __location__)
and otherwise, one gets "implicit definition of talloc_free" and "undefined reference to `talloc_free'".
If you address the pkg-config issue I am happy to inclue your patch. In terms of options.. maybe we can just remove --disable-talloc instead of having sort of a tri-state?
What's the third option?
Jan Engelhardt wrote:
- Firmware builds with no talloc
That may be the use case, but it is not what was implemented.
Why don't you look into fixing that instead of renaming configure options - that's just pointless noise.
If you build 0.7.0 [336915aa0a] with --disable-talloc, `ldd src/.libs/libosmocore.so` will show libtalloc.so.2.
Do you want to investigate why?
+++ b/tests/msgfile/msgfile_test.c @@ -20,6 +20,7 @@ */
#include <osmocom/core/msgfile.h> +#include <osmocom/core/talloc.h>
Is this hunk needed?
Yes, because system talloc has a
#define talloc_free(ctx) _talloc_free(ctx, __location__)
and otherwise, one gets "implicit definition of talloc_free" and "undefined reference to `talloc_free'".
Dude, that is a kludge at a callsite, as opposed to a proper dependency fix. Try again.
//Peter
On Friday 2014-10-03 14:20, Peter Stuge wrote:
Jan Engelhardt wrote:
- Firmware builds with no talloc
That may be the use case, but it is not what was implemented.
Why don't you look into fixing that
Why should I? Why can't you?
If you build 0.7.0 [336915aa0a] with --disable-talloc, `ldd src/.libs/libosmocore.so` will show libtalloc.so.2.
Do you want to investigate why?
Because someone placed -ltalloc into libosmocore_la_LIBADD, the result of satisfying the calls to talloc that the osmocore code is sprinkled with. IOW, osmocore provides no alternative paths to using talloc.
+++ b/tests/msgfile/msgfile_test.c @@ -20,6 +20,7 @@ */
#include <osmocom/core/msgfile.h> +#include <osmocom/core/talloc.h>
Is this hunk needed?
Yes, because system talloc has a
#define talloc_free(ctx) _talloc_free(ctx, __location__)
and otherwise, one gets "implicit definition of talloc_free" and "undefined reference to `talloc_free'".
Dude, that is a kludge at a callsite, as opposed to a proper dependency fix. Try again.
Well, enlighten us, mr-know-it-all.
Jan Engelhardt wrote:
Jan Engelhardt wrote:
- Firmware builds with no talloc
That may be the use case, but it is not what was implemented.
Why don't you look into fixing that
Why should I?
Because you seem to be working with this part of the code now and you might as well do a thorough job if you're spending time on it.
Why can't you?
I wish I could.
+++ b/tests/msgfile/msgfile_test.c @@ -20,6 +20,7 @@ */
#include <osmocom/core/msgfile.h> +#include <osmocom/core/talloc.h>
Is this hunk needed?
Yes, because system talloc has a
#define talloc_free(ctx) _talloc_free(ctx, __location__)
and otherwise, one gets "implicit definition of talloc_free" and "undefined reference to `talloc_free'".
Dude, that is a kludge at a callsite, as opposed to a proper dependency fix. Try again.
Well, enlighten us, mr-know-it-all.
Here's a hint at least: Think about symmetry and dependency.
//Peter
On Friday 2014-10-03 15:16, Peter Stuge wrote:
- Firmware builds with no talloc
That may be the use case, but it is not what was implemented.
Why don't you look into fixing that
Why should I?
Because you seem to be working with this part of the code now and you might as well do a thorough job if you're spending time on it.
I am not going to sprinkle osmocore with defines or otherwise make talloc pluggable/replaceable by some other set of allocation functions. I came to fix the problem I have an interest in, and that is making osmocore support a system talloc in lieu or in addition to a bundled talloc (-> http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries), exercising standard diliegence, which includes that previously supported cases continue to function to the best of observation.
+++ b/tests/msgfile/msgfile_test.c @@ -20,6 +20,7 @@ */
#include <osmocom/core/msgfile.h> +#include <osmocom/core/talloc.h>
Is this hunk needed?
Yes, because system talloc has a #define talloc_free(ctx) _talloc_free(ctx, __location__) and otherwise, one gets "implicit definition of talloc_free" and "undefined reference to `talloc_free'".
Dude, that is a kludge at a callsite, as opposed to a proper dependency fix. Try again.
Here's a hint at least: Think about symmetry and dependency.
I have no idea what you mean by that. Talk code, not riddles. Other code also uses #include <osmocore/core/talloc.h>, so it does not seem too far-fetched to use the exact line to support the case of multiple talloc configurations.
On Fri, Oct 03, 2014 at 04:42:55PM +0200, Jan Engelhardt wrote:
Dear Jan, Peter,
Because you seem to be working with this part of the code now and you might as well do a thorough job if you're spending time on it.
I am not going to sprinkle osmocore with defines or otherwise make talloc pluggable/replaceable by some other set of allocation functions. I came to fix the problem I have an interest in, and that is making osmocore support a system talloc in lieu or in addition to a bundled talloc (-> http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries), exercising standard diliegence, which includes that previously supported cases continue to function to the best of observation.
Nobody is arguing about that. We should not even have a bundled copy of talloc.c. The reason we have one is historic (IIRC there were certainly no -dev/devel packages in any distribution back then and I am not sure if talloc.c even existed outside the samba code).
I do appreciate your work to help us move to using the system version of libtalloc. So let's all calm down.
Is this hunk needed?
Yes, because system talloc has a #define talloc_free(ctx) _talloc_free(ctx, __location__) and otherwise, one gets "implicit definition of talloc_free" and
okay, I see the compiler warning now and will fix it.
cheers holger
Jan Engelhardt wrote:
On Friday 2014-10-03 15:16, Peter Stuge wrote:
- Firmware builds with no talloc
That may be the use case, but it is not what was implemented.
Why don't you look into fixing that
Why should I?
Because you seem to be working with this part of the code now and you might as well do a thorough job if you're spending time on it.
I am not going to
Okey then.
> +#include <osmocom/core/talloc.h>
Is this hunk needed?
Yes, because system talloc has a #define talloc_free(ctx) _talloc_free(ctx, __location__) and otherwise, one gets "implicit definition of talloc_free" and "undefined reference to `talloc_free'".
Dude, that is a kludge at a callsite, as opposed to a proper dependency fix. Try again.
Here's a hint at least: Think about symmetry and dependency.
I have no idea what you mean by that.
That's sad.. :\
Talk code, not riddles. Other code also uses #include <osmocore/core/talloc.h>, so it does not seem too far-fetched to use the exact line to support the case of multiple talloc configurations.
Think more about it.
Why did you add the talloc.h include? - Because the file calls talloc_free().
Why is talloc_free() called? - To free memory allocated and returned by osmo_config_list_parse().
Symmetry then dictates that the talloc.h include belongs in msgfile.h, as opposed to in all files which includes msgfile.h.
I really hope that makes sense. Headers usually do include their dependencies, rather than require callers to do that on their own.
//Peter
On Saturday 2014-10-04 00:45, Peter Stuge wrote:
Why did you add the talloc.h include?
- Because the file calls talloc_free().
Why is talloc_free() called?
- To free memory allocated and returned by osmo_config_list_parse().
Symmetry then dictates that the talloc.h include belongs in msgfile.h, as opposed to in all files which includes msgfile.h.
Certainly. I even concur with the symmetry argument, though I feel you have missed one point:
Symmetry would desire that there be a osmo_config_list_free() function (or macro/static inline function), because the allocation method of "struct osmo_config_list" would be an implementation detail of osmo_config_list_parse().
I really hope that makes sense. Headers usually do include their dependencies, rather than require callers to do that on their own.
As of yet though, talloc is not a "direct" dependency of msgfile.h, that is, there are no names used from talloc.h and `gcc -x c msgfile.h` will succeed.
With a osmo_config_list_free() function/macro/SI, the talloc #include would always be at the callsite too, though the callsite is shifted.
Rename the option --enable-talloc to --enable-internal-talloc, and then make disable-internal-talloc the default when nothing else is specified. --- configure.ac | 12 ++++++------ include/Makefile.am | 2 +- src/Makefile.am | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/configure.ac b/configure.ac index f0d1206..f12675a 100644 --- a/configure.ac +++ b/configure.ac @@ -82,14 +82,14 @@ CHECK_TM_INCLUDES_TM_GMTOFF dnl Generate the output AC_CONFIG_HEADER(config.h)
-AC_ARG_ENABLE(talloc, +AC_ARG_ENABLE([internal-talloc], [AS_HELP_STRING( - [--disable-talloc], + [--enable-internal-talloc], [Disable using bundled talloc memory allocator] )], - [enable_talloc=$enableval], [enable_talloc="yes"]) -AM_CONDITIONAL(ENABLE_TALLOC, [test x"$enable_talloc" = x"yes"]) -AS_IF([test x"$enable_talloc" = xno], + [internal_talloc=$enableval], [internal_talloc="no"]) +AM_CONDITIONAL([INTERNAL_TALLOC], [test x"$internal_talloc" = x"yes"]) +AS_IF([test x"$internal_talloc" = xno], [ PKG_CHECK_MODULES([TALLOC], [talloc]) OSMOCORE_INTERNAL_TALLOC=0 @@ -186,7 +186,7 @@ then AM_CONDITIONAL(ENABLE_MSGFILE, false) AM_CONDITIONAL(ENABLE_SERIAL, false) AM_CONDITIONAL(ENABLE_VTY, false) - AM_CONDITIONAL(ENABLE_TALLOC, false) + AM_CONDITIONAL([INTERNAL_TALLOC], [true]) AM_CONDITIONAL(ENABLE_UTILITIES, false) AM_CONDITIONAL(ENABLE_GB, false) AC_DEFINE([PANIC_INFLOOP],[1],[Use infinite loop on panic rather than fprintf/abort]) diff --git a/include/Makefile.am b/include/Makefile.am index 4d59a72..d8af474 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -90,7 +90,7 @@ nobase_include_HEADERS += osmocom/core/plugin.h endif
nobase_nodist_include_HEADERS = osmocom/core/talloc.h -if ENABLE_TALLOC +if INTERNAL_TALLOC nobase_include_HEADERS += osmocom/core/talloc_int.h endif
diff --git a/src/Makefile.am b/src/Makefile.am index 3924330..4a6ba5c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -27,7 +27,7 @@ else libosmocore_la_LDFLAGS = -version-info $(LIBVERSION) -no-undefined endif
-if ENABLE_TALLOC +if INTERNAL_TALLOC libosmocore_la_SOURCES += talloc.c else libosmocore_la_LIBADD += $(TALLOC_LIBS)
-
Holger Hans Peter Freyther -
Jan Engelhardt -
Peter Stuge