Advanced
Mailing List Archive

Mailing List Archive

  1. Home >
  2. ClamAV>
  3. devel
Making the shared directory a convenience library
steve at lobefin
Aug 22, 2009, 5:24 PM
Post #1 of 4 (1820 views)
Permalink
Hi all,

This patch makes the shared/ subdirectory build a convenience library.
This means that on builds, each object file is only built once, then
compiled into a static, partially linked object file. This has obvious
advantages for the build time, as you only build these files once. It
also has advantages for the maintenance of various Makefile.am's, since
you just include the .la file and let libtool do the rest.

Including inline since I think the list strips attachments.

diff --git a/Makefile.am b/Makefile.am
index 8787f48..4093551 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -18,8 +18,8 @@

ACLOCAL_AMFLAGS=-I m4

-SUBDIRS = libltdl libclamav clamscan clamd clamdscan freshclam sigtool clamconf database docs etc clamav-milter test unit_tests clamdtop clambc
-EXTRA_DIST = FAQ examples BUGS shared libclamav.pc.in libclamunrar_iface/Makefile.am libclamunrar_iface/Makefile.in UPGRADE COPYING.bzip2 COPYING.lzma COPYING.unrar COPYING.LGPL COPYING.file COPYING.zlib COPYING.getopt COPYING.regex COPYING.sha256
+SUBDIRS = libltdl libclamav shared clamscan clamd clamdscan freshclam sigtool clamconf database docs etc clamav-milter test unit_tests clamdtop clambc
+EXTRA_DIST = FAQ examples BUGS libclamav.pc.in libclamunrar_iface/Makefile.am libclamunrar_iface/Makefile.in UPGRADE COPYING.bzip2 COPYING.lzma COPYING.unrar COPYING.LGPL COPYING.file COPYING.zlib COPYING.getopt COPYING.regex COPYING.sha256

bin_SCRIPTS=clamav-config

diff --git a/clamav-milter/Makefile.am b/clamav-milter/Makefile.am
index 5613e53..19e00a2 100644
--- a/clamav-milter/Makefile.am
+++ b/clamav-milter/Makefile.am
@@ -22,14 +22,6 @@ if HAVE_MILTER
sbin_PROGRAMS = clamav-milter

clamav_milter_SOURCES = \
- $(top_srcdir)/shared/optparser.c \
- $(top_srcdir)/shared/optparser.h \
- $(top_srcdir)/shared/output.c \
- $(top_srcdir)/shared/output.h \
- $(top_srcdir)/shared/getopt.c \
- $(top_srcdir)/shared/getopt.h \
- $(top_srcdir)/shared/misc.c \
- $(top_srcdir)/shared/misc.h \
whitelist.c \
whitelist.h \
connpool.c \
@@ -46,6 +38,6 @@ endif

DEFS = @DEFS@ -DCL_NOLIBCLAMAV
CFLAGS=`echo "@CFLAGS@" | sed -e 's/-Wwrite-strings//' -e 's/-Werror /-Werror -Wno-error=format-nonliteral /'`
-LIBS = $(top_builddir)/libclamav/libclamav_internal_utils.la @CLAMAV_MILTER_LIBS@ @THREAD_LIBS@
+LIBS = $(top_builddir)/libclamav/libclamav_internal_utils.la $(top_builddir)/shared/libshared.la @CLAMAV_MILTER_LIBS@ @THREAD_LIBS@
AM_CPPFLAGS = -I$(top_srcdir)/clamd -I$(top_srcdir)/libclamav -I$(top_srcdir)/shared -I$(top_srcdir)
CLEANFILES=*.gcda *.gcno
diff --git a/clambc/Makefile.am b/clambc/Makefile.am
index ff710e2..e9383b1 100644
--- a/clambc/Makefile.am
+++ b/clambc/Makefile.am
@@ -1,13 +1,7 @@
bin_PROGRAMS = clambc
clambc_SOURCES = \
- $(top_srcdir)/shared/optparser.c \
- $(top_srcdir)/shared/optparser.h \
- $(top_srcdir)/shared/getopt.c \
- $(top_srcdir)/shared/getopt.h \
- $(top_srcdir)/shared/misc.c \
- $(top_srcdir)/shared/misc.h \
bcrun.c

AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/shared -I$(top_srcdir)/libclamav
-LIBS = $(top_builddir)/libclamav/libclamav.la @THREAD_LIBS@
+LIBS = $(top_builddir)/libclamav/libclamav.la $(top_builddir)/shared/libshared.la @THREAD_LIBS@
CLEANFILES=*.gcda *.gcno
diff --git a/clamconf/Makefile.am b/clamconf/Makefile.am
index f4d0fe8..619b61e 100644
--- a/clamconf/Makefile.am
+++ b/clamconf/Makefile.am
@@ -19,15 +19,9 @@
bin_PROGRAMS = clamconf

clamconf_SOURCES = \
- $(top_srcdir)/shared/optparser.c \
- $(top_srcdir)/shared/optparser.h \
- $(top_srcdir)/shared/getopt.c \
- $(top_srcdir)/shared/getopt.h \
- $(top_srcdir)/shared/misc.c \
- $(top_srcdir)/shared/misc.h \
clamconf.c

DEFS = @DEFS@ -DCL_NOTHREADS
AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/shared -I$(top_srcdir)/libclamav
-LIBS = $(top_builddir)/libclamav/libclamav.la @CLAMD_LIBS@ @THREAD_LIBS@
+LIBS = $(top_builddir)/libclamav/libclamav.la $(top_builddir)/shared/libshared.la @CLAMD_LIBS@ @THREAD_LIBS@
CLEANFILES=*.gcda *.gcno
diff --git a/clamd/Makefile.am b/clamd/Makefile.am
index 573ff64..a3ca414 100644
--- a/clamd/Makefile.am
+++ b/clamd/Makefile.am
@@ -21,16 +21,6 @@ if BUILD_CLAMD
sbin_PROGRAMS = clamd

clamd_SOURCES = \
- $(top_srcdir)/shared/output.c \
- $(top_srcdir)/shared/output.h \
- $(top_srcdir)/shared/optparser.c \
- $(top_srcdir)/shared/optparser.h \
- $(top_srcdir)/shared/getopt.c \
- $(top_srcdir)/shared/getopt.h \
- $(top_srcdir)/shared/misc.c \
- $(top_srcdir)/shared/misc.h \
- $(top_srcdir)/shared/network.c \
- $(top_srcdir)/shared/network.h \
clamd.c \
tcpserver.c \
tcpserver.h \
@@ -58,7 +48,7 @@ clamd_SOURCES = \

endif

-LIBS = $(top_builddir)/libclamav/libclamav.la @CLAMD_LIBS@ @THREAD_LIBS@
+LIBS = $(top_builddir)/libclamav/libclamav.la $(top_builddir)/shared/libshared.la @CLAMD_LIBS@ @THREAD_LIBS@
AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/shared -I$(top_srcdir)/libclamav

# it does support --help and --version but with the default config file
diff --git a/clamdscan/Makefile.am b/clamdscan/Makefile.am
index 8a58219..cace364 100644
--- a/clamdscan/Makefile.am
+++ b/clamdscan/Makefile.am
@@ -21,17 +21,6 @@ if BUILD_CLAMD
bin_PROGRAMS = clamdscan

clamdscan_SOURCES = \
- $(top_srcdir)/shared/output.c \
- $(top_srcdir)/shared/output.h \
- $(top_srcdir)/shared/optparser.c \
- $(top_srcdir)/shared/optparser.h \
- $(top_srcdir)/shared/misc.c \
- $(top_srcdir)/shared/misc.h \
- $(top_srcdir)/shared/getopt.c \
- $(top_srcdir)/shared/getopt.h \
- $(top_srcdir)/shared/actions.c \
- $(top_srcdir)/shared/actions.h \
- $(top_srcdir)/libclamav/regex/strlcpy.c\
clamdscan.c \
proto.c \
proto.h \
@@ -41,7 +30,7 @@ endif

DEFS = @DEFS@ -DCL_NOTHREADS -DCL_NOLIBCLAMAV
AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/clamscan -I$(top_srcdir)/shared -I$(top_srcdir)/libclamav
-LIBS = $(top_builddir)/libclamav/libclamav_internal_utils_nothreads.la @FRESHCLAM_LIBS@
+LIBS = $(top_builddir)/libclamav/libclamav_internal_utils_nothreads.la $(top_builddir)/shared/libshared.la @FRESHCLAM_LIBS@

AM_INSTALLCHECK_STD_OPTIONS_EXEMPT=clamdscan$(EXEEXT)
CLEANFILES=*.gcda *.gcno
diff --git a/clamdtop/Makefile.am b/clamdtop/Makefile.am
index f3ed3e5..c76f053 100644
--- a/clamdtop/Makefile.am
+++ b/clamdtop/Makefile.am
@@ -2,16 +2,10 @@ if HAVE_CURSES
bin_PROGRAMS = clamdtop
man_MANS = $(top_builddir)/docs/man/clamdtop.1
clamdtop_SOURCES = \
- $(top_srcdir)/shared/optparser.c \
- $(top_srcdir)/shared/optparser.h \
- $(top_srcdir)/shared/getopt.c \
- $(top_srcdir)/shared/getopt.h \
- $(top_srcdir)/shared/misc.c \
- $(top_srcdir)/shared/misc.h \
clamdtop.c

AM_CPPFLAGS = -I$(top_srcdir) @CURSES_CPPFLAGS@
-clamdtop_LDADD = @CURSES_LIBS@ $(top_builddir)/libclamav/libclamav_internal_utils_nothreads.la
+clamdtop_LDADD = @CURSES_LIBS@ $(top_builddir)/libclamav/libclamav_internal_utils_nothreads.la $(top_builddir)/shared/libshared.la
endif
DEFS = @DEFS@ -DCL_NOTHREADS -DCL_NOLIBCLAMAV
EXTRA_DIST = clamdtop.c
diff --git a/clamscan/Makefile.am b/clamscan/Makefile.am
index c965af4..a296ebe 100644
--- a/clamscan/Makefile.am
+++ b/clamscan/Makefile.am
@@ -20,16 +20,6 @@
bin_PROGRAMS = clamscan

clamscan_SOURCES = \
- $(top_srcdir)/shared/output.c \
- $(top_srcdir)/shared/output.h \
- $(top_srcdir)/shared/getopt.c \
- $(top_srcdir)/shared/getopt.h \
- $(top_srcdir)/shared/optparser.c \
- $(top_srcdir)/shared/optparser.h \
- $(top_srcdir)/shared/actions.c \
- $(top_srcdir)/shared/actions.h \
- $(top_srcdir)/shared/misc.c \
- $(top_srcdir)/shared/misc.h \
clamscan.c \
others.c \
others.h \
@@ -38,7 +28,7 @@ clamscan_SOURCES = \
manager.h

DEFS = @DEFS@ -DCL_NOTHREADS
-LIBS = $(top_builddir)/libclamav/libclamav.la @THREAD_LIBS@
+LIBS = $(top_builddir)/libclamav/libclamav.la $(top_builddir)/shared/libshared.la @THREAD_LIBS@
AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/shared -I$(top_srcdir)/libclamav

CLEANFILES=*.gcda *.gcno
diff --git a/configure.in b/configure.in
index 7fbcfc2..9f63734 100644
--- a/configure.in
+++ b/configure.in
@@ -1596,6 +1596,7 @@ test/Makefile
unit_tests/Makefile
clamdtop/Makefile
clambc/Makefile
+shared/Makefile
Makefile
clamav-config
libclamav.pc
diff --git a/freshclam/Makefile.am b/freshclam/Makefile.am
index 7b691f4..30c3367 100644
--- a/freshclam/Makefile.am
+++ b/freshclam/Makefile.am
@@ -20,18 +20,6 @@
bin_PROGRAMS = freshclam

freshclam_SOURCES = \
- $(top_srcdir)/shared/output.c \
- $(top_srcdir)/shared/output.h \
- $(top_srcdir)/shared/optparser.c \
- $(top_srcdir)/shared/optparser.h \
- $(top_srcdir)/shared/getopt.c \
- $(top_srcdir)/shared/getopt.h \
- $(top_srcdir)/shared/misc.c \
- $(top_srcdir)/shared/misc.h \
- $(top_srcdir)/shared/cdiff.c \
- $(top_srcdir)/shared/cdiff.h \
- $(top_srcdir)/shared/tar.c \
- $(top_srcdir)/shared/tar.h \
freshclam.c \
manager.c \
manager.h \
@@ -48,7 +36,7 @@ freshclam_SOURCES = \

DEFS = @DEFS@ -DCL_NOTHREADS
AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/shared -I$(top_srcdir)/libclamav
-LIBS = $(top_builddir)/libclamav/libclamav.la @FRESHCLAM_LIBS@ @THREAD_LIBS@
+LIBS = $(top_builddir)/libclamav/libclamav.la $(top_builddir)/shared/libshared.la @FRESHCLAM_LIBS@ @THREAD_LIBS@

AM_INSTALLCHECK_STD_OPTIONS_EXEMPT=freshclam$(EXEEXT)
CLEANFILES=*.gcda *.gcno
diff --git a/sigtool/Makefile.am b/sigtool/Makefile.am
index 3c4180b..7c8d041 100644
--- a/sigtool/Makefile.am
+++ b/sigtool/Makefile.am
@@ -19,23 +19,11 @@
bin_PROGRAMS = sigtool

sigtool_SOURCES = \
- $(top_srcdir)/shared/output.c \
- $(top_srcdir)/shared/output.h \
- $(top_srcdir)/shared/getopt.c \
- $(top_srcdir)/shared/getopt.h \
- $(top_srcdir)/shared/optparser.c \
- $(top_srcdir)/shared/optparser.h \
- $(top_srcdir)/shared/misc.c \
- $(top_srcdir)/shared/misc.h \
- $(top_srcdir)/shared/cdiff.c \
- $(top_srcdir)/shared/cdiff.h \
- $(top_srcdir)/shared/tar.c \
- $(top_srcdir)/shared/tar.h \
vba.c \
vba.h \
sigtool.c

DEFS = @DEFS@ -DCL_NOTHREADS
AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/shared -I$(top_srcdir)/libclamav
-LIBS = $(top_builddir)/libclamav/libclamav.la @FRESHCLAM_LIBS@ @THREAD_LIBS@
+LIBS = $(top_builddir)/libclamav/libclamav.la $(top_builddir)/shared/libshared.la @FRESHCLAM_LIBS@ @THREAD_LIBS@
CLEANFILES=*.gcda *.gcno

Cheers,
--
--------------------------------------------------------------------------
| Stephen Gran | Do not underestimate the value of print |
| steve@lobefin.net | statements for debugging. Don't have |
| http://www.lobefin.net/~steve | aesthetic convulsions when using them, |
| | either. |
--------------------------------------------------------------------------

Attached Files:

Re: Making the shared directory a convenience library [ In reply to ]
edwintorok at gmail
Aug 24, 2009, 3:06 AM
Post #2 of 4 (1753 views)
Permalink
On 2009-08-23 03:24, Stephen Gran wrote:
> Hi all,
>
> This patch makes the shared/ subdirectory build a convenience library.
>

Hi Stephen,

Did you git add shared/Makefile.am? I don't see it in your patch.

> This means that on builds, each object file is only built once, then
> compiled into a static, partially linked object file. This has obvious
> advantages for the build time, as you only build these files once. It
> also has advantages for the maintenance of various Makefile.am's, since
> you just include the .la file and let libtool do the rest.
>
> Including inline since I think the list strips attachments.shar
>
>

There are some ifdefs in shared/, CL_NOLIBCLAMAV, and CL_NOTHREADS, with
your patch
all the code would be built as if those would not be defined.

NOLIBCLAMAV is needed by clamav-milter, clamdscan, and clamdtop. I
t is particularly important for clamdscan, we don't want clamdscan to
link libclamav!

AFAICT the NOTHREADS define is only needed to avoid linking programs
that don't need pthread with pthread.

I'm not particularly fond of the way NOLIBCLAMAV is handled, maybe it
would be better to split mis.c into two files, and have
2 shared.la files built: shared_nolibclamav.la, shared_libclamav.la, and
have clamdscan link only shared_nolibclamav.la.

Not sure what should be done about NOTHREADS, but I think currently
these permutations are needed:
shared_nolibclamav
shared_libclamav
shared_threads_libclamav
shared_threads_nolibclamav

Not all files in shared/ use those defines, so you may avoid rebuilding
each with different CFLAGS.


> diff --git a/clamav-milter/Makefile.am b/clamav-milter/Makefile.am
> index 5613e53..19e00a2 100644
> --- a/clamav-milter/Makefile.am
> +++ b/clamav-milter/Makefile.am
> @@ -22,14 +22,6 @@ if HAVE_MILTER
> sbin_PROGRAMS = clamav-milter
>
> clamav_milter_SOURCES = \
> - $(top_srcdir)/shared/optparser.c \
> - $(top_srcdir)/shared/optparser.h \
> - $(top_srcdir)/shared/output.c \
> - $(top_srcdir)/shared/output.h \
> - $(top_srcdir)/shared/getopt.c \
> - $(top_srcdir)/shared/getopt.h \
> - $(top_srcdir)/shared/misc.c \
> - $(top_srcdir)/shared/misc.h \
> whitelist.c \
> whitelist.h \
> connpool.c \
> @@ -46,6 +38,6 @@ endif
>
> DEFS = @DEFS@ -DCL_NOLIBCLAMAV
>
Here is one such def.

> diff --git a/clamscan/Makefile.am b/clamscan/Makefile.am
> index c965af4..a296ebe 100644
> --- a/clamscan/Makefile.am
> +++ b/clamscan/Makefile.am
> @@ -20,16 +20,6 @@
> bin_PROGRAMS = clamscan
>
> clamscan_SOURCES = \
> - $(top_srcdir)/shared/output.c \
> - $(top_srcdir)/shared/output.h \
> - $(top_srcdir)/shared/getopt.c \
> - $(top_srcdir)/shared/getopt.h \
> - $(top_srcdir)/shared/optparser.c \
> - $(top_srcdir)/shared/optparser.h \
> - $(top_srcdir)/shared/actions.c \
> - $(top_srcdir)/shared/actions.h \
> - $(top_srcdir)/shared/misc.c \
> - $(top_srcdir)/shared/misc.h \
> clamscan.c \
> others.c \
> others.h \
> @@ -38,7 +28,7 @@ clamscan_SOURCES = \
> manager.h
>
> DEFS = @DEFS@ -DCL_NOTHREADS
>
And here is another.

Best regards,
--Edwin
_______________________________________________
http://lurker.clamav.net/list/clamav-devel.html
Please submit your patches to our Bugzilla: http://bugs.clamav.net
Re: Making the shared directory a convenience library [ In reply to ]
steve at lobefin
Aug 24, 2009, 2:13 PM
Post #3 of 4 (1750 views)
Permalink
On Mon, Aug 24, 2009 at 01:06:10PM +0300, Török Edwin said:
> On 2009-08-23 03:24, Stephen Gran wrote:
> > Hi all,
> >
> > This patch makes the shared/ subdirectory build a convenience library.
>
> Hi Stephen,
>
> Did you git add shared/Makefile.am? I don't see it in your patch.

Arg, I must not have. Sorry about that. It boiled down to a very
simplistic:

noinst_LTLIBRARIES = libshared.la
libshared_la_SOURCES = <bunch of source files>
LIBS = $(top_sourcedir)/libclamav/libclamav.la

> > This means that on builds, each object file is only built once, then
> > compiled into a static, partially linked object file. This has obvious
> > advantages for the build time, as you only build these files once. It
> > also has advantages for the maintenance of various Makefile.am's, since
> > you just include the .la file and let libtool do the rest.
> >
> > Including inline since I think the list strips attachments.shar
>
> There are some ifdefs in shared/, CL_NOLIBCLAMAV, and CL_NOTHREADS, with
> your patch
> all the code would be built as if those would not be defined.

Right. I think what was underlying my rather simple minded approach was
a general feeling that some of the things being done for linker
optimization in clamav might be unnecessary.

The c library on all the POSIX platforms I'm familiar with provide stub
functions for the pthread functions that are weak symbols and can be
masked out by a thread library when linked to one, but can be effectively
no-ops otherwise. Consider this simple bit of c:

#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

int main (void) {
int x = 0;
pthread_mutex_t mut = PTHREAD_MUTEX_INITIALIZER;
pthread_mutex_lock(&mut);
x++;
pthread_mutex_unlock(&mut);
printf("%d\n", x);
exit(0);
}

It compiles, links and runs whether or not I add a thread library to
the link line. Are there POSIX platforms that this doesn't work on?
If (as I hope and suspect) not, can't the same principle be applied in
clamav - let the link line of the application being compiled determine
whether these pthread symbols are relocated to the thread library or
left as stubs in the c library? You don't need to link -lshared to
-lpthread (or -lthread, or ....) - just link clamd to the thread library.
As you're effectively objcopying in symbols from libshared.a at that
point, the linker should deal with it correctly. No guarantees, of
course, I realize there are plenty of broken platforms out there.

> NOLIBCLAMAV is needed by clamav-milter, clamdscan, and clamdtop. I
> t is particularly important for clamdscan, we don't want clamdscan to
> link libclamav!

This is another one I'm not sure I understand the importance of. If
there are no symbols being used from libclamav, the relocation overhead
at startup is pretty minimal. I agree you may be able to shave a few
milliseconds off of startup time by not having a .NEEDED entry for
libclamav at all, but it hardly seems worth the effort. I may be
missing something obvious though, so feel free to correct me.

> I'm not particularly fond of the way NOLIBCLAMAV is handled, maybe it
> would be better to split mis.c into two files, and have
> 2 shared.la files built: shared_nolibclamav.la, shared_libclamav.la, and
> have clamdscan link only shared_nolibclamav.la.
>
> Not sure what should be done about NOTHREADS, but I think currently
> these permutations are needed:
> shared_nolibclamav
> shared_libclamav
> shared_threads_libclamav
> shared_threads_nolibclamav
>
> Not all files in shared/ use those defines, so you may avoid rebuilding
> each with different CFLAGS.

That way seems to lie combinatorial madness. The current solution is
far better than that :)

Cheers,
--
--------------------------------------------------------------------------
| Stephen Gran | I don't like this official/unofficial |
| steve@lobefin.net | distinction. It sound, er, officious. |
| http://www.lobefin.net/~steve | -- Larry Wall in |
| | <199702221943.LAA20388@wall.org> |
--------------------------------------------------------------------------

Attached Files:

Re: Making the shared directory a convenience library [ In reply to ]
edwintorok at gmail
Aug 25, 2009, 12:26 AM
Post #4 of 4 (1747 views)
Permalink
On 2009-08-25 00:13, Stephen Gran wrote:
> On Mon, Aug 24, 2009 at 01:06:10PM +0300, Török Edwin said:
>
>> On 2009-08-23 03:24, Stephen Gran wrote:
>>
>>> Hi all,
>>>
>>> This patch makes the shared/ subdirectory build a convenience library.
>>>
>> Hi Stephen,
>>
>> Did you git add shared/Makefile.am? I don't see it in your patch.
>>
>
> Arg, I must not have. Sorry about that. It boiled down to a very
> simplistic:
>
> noinst_LTLIBRARIES = libshared.la
> libshared_la_SOURCES = <bunch of source files>
> LIBS = $(top_sourcedir)/libclamav/libclamav.la
>

Ok.

>
>>> This means that on builds, each object file is only built once, then
>>> compiled into a static, partially linked object file. This has obvious
>>> advantages for the build time, as you only build these files once. It
>>> also has advantages for the maintenance of various Makefile.am's, since
>>> you just include the .la file and let libtool do the rest.
>>>
>>> Including inline since I think the list strips attachments.shar
>>>
>> There are some ifdefs in shared/, CL_NOLIBCLAMAV, and CL_NOTHREADS, with
>> your patch
>> all the code would be built as if those would not be defined.
>>
>
> Right. I think what was underlying my rather simple minded approach was
> a general feeling that some of the things being done for linker
> optimization in clamav might be unnecessary.
>
> The c library on all the POSIX platforms I'm familiar with provide stub
> functions for the pthread functions that are weak symbols and can be
> masked out by a thread library when linked to one, but can be effectively
> no-ops otherwise. Consider this simple bit of c:
>
> #include <pthread.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> int main (void) {
> int x = 0;
> pthread_mutex_t mut = PTHREAD_MUTEX_INITIALIZER;
> pthread_mutex_lock(&mut);
> x++;
> pthread_mutex_unlock(&mut);
> printf("%d\n", x);
> exit(0);
> }
>
> It compiles, links and runs whether or not I add a thread library to
> the link line. Are there POSIX platforms that this doesn't work on?
>

AIX for example:

$ gcc x.c
ld: 0711-317 ERROR: Undefined symbol: .pthread_mutex_lock
ld: 0711-317 ERROR: Undefined symbol: .pthread_mutex_unlock
ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more
information.
collect2: ld returned 8 exit status

I wouldn't use the pthread symbols if I don't use -pthread or -lpthread.

Perhaps those programs that don't need pthreads (clamscan, freshclam) could
either link to pthread, or link a special .c file in shared.c that
defines pthread_mutex_lock/unlock
as dummy noop functions.

> If (as I hope and suspect) not, can't the same principle be applied in
> clamav - let the link line of the application being compiled determine
> whether these pthread symbols are relocated to the thread library or
> left as stubs in the c library? You don't need to link -lshared to
> -lpthread (or -lthread, or ....) - just link clamd to the thread library.
> As you're effectively objcopying in symbols from libshared.a at that
> point, the linker should deal with it correctly. No guarantees, of
> course, I realize there are plenty of broken platforms out there.
>

For clamd this is not a problem, .a libraries don't need to be linked
with pthread.
It is rather something for clamscan and freshclam.

>
>> NOLIBCLAMAV is needed by clamav-milter, clamdscan, and clamdtop. I
>> t is particularly important for clamdscan, we don't want clamdscan to
>> link libclamav!
>>
>
> This is another one I'm not sure I understand the importance of. If
> there are no symbols being used from libclamav, the relocation overhead
> at startup is pretty minimal.

The milter talks to clamd, if its linked to libclamav then it might need
to be upgraded everytime
libclamav is upgraded with a different soname. That seems unnecessary.

Also in the past clamdscan was using some string function from
libclamav, so it didn't work without libclamav.
I thought it'd also be nicer for packagers since they can put clamdscan
in a package that doesn't depend on libclamav.

> I agree you may be able to shave a few
> milliseconds off of startup time by not having a .NEEDED entry for
> libclamav at all, but it hardly seems worth the effort.

Its already been done, so why undo an improvement? :)

You could have a shared_nolibclamav that contains the .c files that
don't link to libclamav, and
a shared_libclamav that contains the .c files that do link to it.
Then clamd will use both .la files, clamscan&freshclam will only use one.

> I may be
> missing something obvious though, so feel free to correct me.
>

libclamav also pulls in libz, libdl, libbz2, libpthread.
clamdscan uses 19M VIRT, vs 10M VIRT currently, vs 6M VIRT if I further
reduce the dependencies.

>
>> I'm not particularly fond of the way NOLIBCLAMAV is handled, maybe it
>> would be better to split mis.c into two files, and have
>> 2 shared.la files built: shared_nolibclamav.la, shared_libclamav.la, and
>> have clamdscan link only shared_nolibclamav.la.
>>
>> Not sure what should be done about NOTHREADS, but I think currently
>> these permutations are needed:
>> shared_nolibclamav
>> shared_libclamav
>> shared_threads_libclamav
>> shared_threads_nolibclamav
>>
>> Not all files in shared/ use those defines, so you may avoid rebuilding
>> each with different CFLAGS.
>>
>
> That way seems to lie combinatorial madness. The current solution is
> far better than that :)
>

I think nothreads is not that important, it can be worked around as
suggested above (link a .c file that defines dummy functions).

Best regards,
--Edwin
_______________________________________________
http://lurker.clamav.net/list/clamav-devel.html
Please submit your patches to our Bugzilla: http://bugs.clamav.net

©2024 GT.net. A Gossamer Threads company.
5th Floor, 455 Granville St., Vancouver, BC V6C 1T1, Canada | Legal