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 |
| [email protected] | distinction. It sound, er, officious. |
| http://www.lobefin.net/~steve | -- Larry Wall in |
| | <[email protected]> |
--------------------------------------------------------------------------
signature.asc
Description: Digital signature
_______________________________________________ http://lurker.clamav.net/list/clamav-devel.html Please submit your patches to our Bugzilla: http://bugs.clamav.net
