Hello,

commit "convert com_err/et/libcom_err.a to a libtool archive"

You should also fix the 2nd argument to db_panic() in lib/cyrusdb_berkeley.c.

I am not sure: gcc does not complain.  I think it matters only when
errno is a global variable, in db_panic() it is local.

Many systems define of errno as

extern int errno;

but that's not the only possibility; on some platforms under some
combinations of compiler flags errno is a macro that calls a function
that returns a pointer to a variable which might or might not be
thread-specific at runtime.  See

http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/head/errno.h

55 extern int *___errno();
56 #define      errno (*(___errno()))

Under these conditions, using errno as the name of a function parameter
gets you, if you're lucky, a compile error because the function
definition doesn't match an earlier declaration.  If you're unlucky or
silly, you end up with a function which you thought was defined

int foo(int errno) { ... }

but was actually defined

int foo(int (*___errno())) { ... }

which compiles but is taking a different type of argument to the one you
thought it was taking.  Furthermore it is hiding the global function
___errno() with a parameter declaration of pointer-to-function type, so
any use of errno in the function is going to call through that function
pointer instead of calling the global function. The value passed for
that parameter will be from a caller which thinks that the type of the
parameter is an int, and will be a small integer error number which will
most definitely not be a valid pointer to a function.  The result is
that any use of the parameter in the function will SEGV.

In this specific case, we're getting away with it because

a) db_panic() doesn't use it's errno argument anywhere, and

b) db_panic() is almost never called, and when it is the situation is so
dire that exit() is the only option anyway, so SEGVing here would go
unnoticed and unfixed, and

c) in the one place where db_panic() is used, it's explicitly cast to
the type the author was expecting

static void db_panic(DB_ENV *dbenv __attribute__((unused)),
                     int errno __attribute__((unused)))
...
     dbenv->set_paniccall(dbenv, (void (*)(DB_ENV *, int))&db_panic);

which suppress any compile error.  I bet someone discovered that compile
error and "fixed" it with a cast.

I think renaming "errno" to "noerr" in lib/cyrus_berkeley.c:db_panic resolves the issue with the macros and has no other impacts.

Also, I notice that libcom_err.la is 'noinst'.  How do you expect that cyrus 
executable will be able to find this code at runtime on a platform which has 
shared libraries but no system com_err library?

When the compilation of the internal com_err is requested, the files
from libcom_err.la are compiled with -fPIC and statically linked in
libimap and libsieve.

Two questions -

1) is this happening by accident because we're currently adding -fPIC to
the global CFLAGS via @PERL_CCLDFLAGS@ ?

According to my understanding, -fPIC is added, so that the compiled .o files, can be inserted in a shared library and this is independent from @PERL_CCLDFLAGS@.


2) would it not be better to bundle the libcom_err code into libcyrus.so
?

This would require imtest/imtest, netnews/remotepurge and notifyd/notify to load code for com_err, which they do not use.

That is how the files are found at run time.
This has the disadvantage, that running timsieved and lmtpd with shared
libraries support, loads libimap and libsieve, so the statically linked
code of libcim_err is loaded twice.  An option would be to install
com_err.la as shared libcyrus_com_err, which is loaded by both libimap
and libsieve.

That would also work.

Със здраве
  Дилян

<<attachment: dilyan_palauzov.vcf>>

Reply via email to