Stephen Gran wrote: > Hi all, > > So I've been thinking about how to reduce the breakage that occasionally > happens to other bits of software on upgrades of libclamav, and I think > I've come up with something that might be of some use. libtool lets you > use a version script to (on platforms that support it) hide symbols from > being globally visible and version those that are. > > The benefits of using this sort of approach is that other applications > that try to use libclamav internal functions will fail to link instead > of just throwing a compiler warning about an implicit function > definition. This will allow the clamav team to make their public API > more stable, since it is the only thing other people will be using. > > The downfall, as I have just found out by setting up a version script, > is the the clamav applications fail to link without exporting quite a > few symbols that really shouldn't be exported by the library.
This reminds me of https://wwws.clamav.net/bugzilla/show_bug.cgi?id=272 > It also > doesn't actually help with API/ABI changing modifications in the public > API itself - Ulrich Drepper says in his DSO howto (http://people.redhat.com/drepper/dsohowto.pdf) that API/ABI stability can be maintained via versioning. But that is stuff for post 1.0 IMHO. > it just frees you to spend more time making sure that can > be left alone while you modify the private functions to your heart's > content. > Did any breakage occur due to changes in cli_* functions? People should really not use those directly. > This says to me that probably the architecture for some of this shared > code is wrong at the moment. One of the following solutions is probably > useful, but I'll leave it up to you all as to which one you prefer. > > 1) these symbols should be useable by all applications linking to > libclamav (and they should be cl_ instead of their current naming > scheme). > > 2) These symbols should not be in libclamav at all. They should be code > in shared/ which gets turned into a convenience library and linked to > all the applications and the library at build time, presumably > statically. > > 3) These symbols should be in a second shared library built from shared/ > or elsewhere that is installed on the target system. To discourage use > of this private library, it could be installed in a sub directory of > $libdir, and an rpath added to all the binaries and libclamav. > > My personal preference is 2. It incurs some on disk and memory > overhead, as these symbols will be loaded once for each application, > rather than the normal shared library memory saving techniques. That > being said, it is the simplest to manage, and is the most effective at > making sure other applications do not link to private functions within > the clamav suite that change between releases. > Actually if we have export maps the size of the .so will be cut down, so maybe it would compensate for it. > Ok, so long preamble over. > > version script: > [.....] > vba56_dir_read; > vba_decompress; > I guess this is only used by sigtool. I wouldn't mind linking sigtool statically with libclamav, it is not a tool used on a day-by-day basis by end-users. I think if you don't consider sigtool you can significantly shorten this list. Besides if people see this list in an export map they'll start using it, because they'll consider it public :( Also clamdscan has more dependencies than it should. > wm_decrypt_macro; > wm_dir_read; > local: > *; > }; > > Everything in global: is exported and useable at link time. I was also thinking of using -fvisibility feature of gcc to hide internals functions, which has the nice benefit of avoiding an indirect call (usually symbols not declared static need an extra indirection due to -fPIC). However I didn't like the way -fvisibility has to be used. I either have to explicitly declare the visibility of all functions exported, or declare visibility on all functions being hidden. Even on functions declared as extern! Sure, we could do an EXTERN macro that also adds the visibility attribute ... However linker export maps can be implemented independently of -fivisibility :) > Everything > in local is not. As you can see, you can use wildcards, and you can > also chain versions, so that if a symbol is introduced or removed in a > later version of the library, it gets versioned correctly. All very > nice stuff. > > The automake patches: > Index: libclamav/Makefile.am > =================================================================== > --- libclamav/Makefile.am (revision 572) > +++ libclamav/Makefile.am (working copy) > @@ -21,8 +21,10 @@ > > libclamav_la_LIBADD = @LIBCLAMAV_LIBS@ @THREAD_LIBS@ > > -libclamav_la_LDFLAGS = @TH_SAFE@ -version-info @LIBCLAMAV_VERSION@ > -no-undefined > +libclamav_la_LDFLAGS = @TH_SAFE@ -version-info @LIBCLAMAV_VERSION@ > -no-undefined -Wl,[EMAIL PROTECTED]@/libclamav/libclamav.map > > +EXTRA_DIST = libclamav.map > + > include_HEADERS = clamav.h > > libclamav_la_SOURCES = \ > Index: libclamav/Makefile.in > =================================================================== > --- libclamav/Makefile.in (revision 572) > +++ libclamav/Makefile.in (working copy) > @@ -238,7 +238,8 @@ > target_vendor = @target_vendor@ > INCLUDES = -I$(top_srcdir) [EMAIL PROTECTED]@/unrar [EMAIL PROTECTED]@/nsis > libclamav_la_LIBADD = @LIBCLAMAV_LIBS@ @THREAD_LIBS@ > -libclamav_la_LDFLAGS = @TH_SAFE@ -version-info @LIBCLAMAV_VERSION@ > -no-undefined > +libclamav_la_LDFLAGS = @TH_SAFE@ -version-info @LIBCLAMAV_VERSION@ > -no-undefined -Wl,[EMAIL PROTECTED]@/libclamav/libclamav.map > +EXTRA_DIST = libclamav.map > include_HEADERS = clamav.h > libclamav_la_SOURCES = \ > clamav.h \ > > That's about it. This is against current stable, although rerevving it > against svn would be fairly trivial. > > Is this something people are interested in? I wanted to do something similar a long while ago, so I am definetely interested in helping out. > If so, how can I help to > get the list down to > > global: > cl_*; > local: > *; > > ? > Yes it would be nice to narrow down the list to this. Does this work with GNU ld only? (it is supposed to work with Solaris's ld too) Best regards, --Edwin _______________________________________________ http://lurker.clamav.net/list/clamav-devel.html Please submit your patches to our Bugzilla: http://bugs.clamav.net
