[PATCH xdm] Re-introduce the ability of staticcally linking the greeter.
Matthieu Herrb
matthieu.herrb at laas.fr
Tue Nov 8 15:34:18 PST 2011
On Tue, Nov 08, 2011 at 04:05:06PM -0500, Gaetan Nadon wrote:
> On Mon, 2011-11-07 at 19:57 +0000, Matthieu Herrb wrote:
>
> > This logically reverts 7e223d3ac6c0d549a7d6e4dcc86a053e19594028.
> > There are still cases (in particular OpenBSD) where the shared greeter
> > is not desired.
>
> My reading at the time was the static version was there because some O/S
> did not have loadable libraries support (this greeter seems to be very
> old code). Can you add in the commit text (if you know) the reasons why
> OpenBSD would not be able to use a shared library? Less chances of being
> removed in a few years from now. I thought all platforms could use
> shared libraries.
OpenBSD/vax (don't laugh please) is still a.out and doesn't have
shared libs. Yet it can run xdm (no X server though since the ability
of statically linking the drivers was lost a few years ago :().
But this is not the main reason. Basically the shared object doesn't
give us any benefit. So why bother with the extra complexity and
failures cases that it introduces ?
>
> >
> > BTW, Ubuntu 11.10 and Fedora 15 both ship xdm 1.10 with the shared
> > greeter disabled too.
>
> And we don't if there are reasons for that or if it is just an old build
> script that works...
As Alan pointed out, there is no real reason to keep the shared object
version as long as no one provides alternate greeters that could be
plugged in to replace the default one.
If you want to shrink the source, I would rather drop the support for
the dynamic greeter.
>
> >
> > Signed-off-by: Matthieu Herrb <matthieu.herrb at laas.fr>
> > ---
> > configure.ac | 28 ++++++++++++++++++++++++++--
> > greeter/Makefile.am | 8 ++++++++
> > greeter/greet.c | 6 ++++--
> > xdm/Makefile.am | 5 ++++-
> > xdm/session.c | 12 +++++++++++-
> > 5 files changed, 53 insertions(+), 6 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 18848e8..7d41ce7 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -419,6 +419,30 @@ PKG_CHECK_MODULES(AUTH, xau)
> > # Greeter
> > #
> >
> > +AC_MSG_CHECKING([if the greeter should be built as a dynamically loaded object])
> > +
> > +AC_ARG_ENABLE(dynamic-greeter,
> > + AC_HELP_STRING([--enable-dynamic-greeter],
> > + [Build greeter as dynamically loaded shared object]),
> > + [DYNAMIC_GREETER="$enableval"], [DYNAMIC_GREETER="auto"])
> > +
> > +if test "x$DYNAMIC_GREETER" = "xauto" ; then
> > + case $host_os in
> > + openbsd*)
> > + DYNAMIC_GREETER="no"
> > + ;;
> > + *) DYNAMIC_GREETER="yes" ;;
> > +
> > + esac
> > +fi
> > +if test "x$DYNAMIC_GREETER" = "xno"; then
> > +STATIC_GREETER_CFLAGS="-DGREET_USER_STATIC"
> > +STATIC_GREETER_LIBS="-L../greeter -lXdmGreet"
>
> This isn't the correct way of using a library built by the module. It
> assumes it is installed. The "../" is always bad news in a makefile. I
> recall going through this scenario in libpciaccess with the help of
> reviewers and the final code turned out to be:
>
> LDADD = $(top_builddir)/src/libpciaccess.la
>
> where the scanpci program needed to be linked with libpciaccess. The
> ".la" file should be used.
> The -L essentially bypasses Automake and is being added as a search path
> for other libraries which could link to the wrong symbol if it happens
> to be defined in more than one library.
>
>
> > +fi
> > +
> > +AC_MSG_RESULT([$DYNAMIC_GREETER])
> > +AM_CONDITIONAL(DYNAMIC_GREETER, test x$DYNAMIC_GREETER = xyes)
> > +
> > PKG_CHECK_MODULES(XDMGREET, xt >= 1.0 x11 xext)
> >
> > GREETERLIB="${XDMLIBDIR}/libXdmGreet.so"
> > @@ -431,8 +455,8 @@ XDMGREET_LIBS="$XDMGREET_LIBS $XDM_TOOLKIT_LIBS $DMCP_LIBS $GREETER_LIBS"
> > # XDM
> > #
> >
> > -XDM_CFLAGS="$XDM_CFLAGS $DMCP_CFLAGS $XLIB_CFLAGS $AUTH_CFLAGS"
> > -XDM_LIBS="$XDM_LIBS $DMCP_LIBS"
> > +XDM_CFLAGS="$XDM_CFLAGS $DMCP_CFLAGS $XLIB_CFLAGS $AUTH_CFLAGS $STATIC_GREETER_CFLAGS"
> > +XDM_LIBS="$XDM_LIBS $DMCP_LIBS $STATIC_GREETER_LIBS"
> >
> > AC_CHECK_LIB(Xdmcp, XdmcpWrap, [xdmauth="yes"], [xdmauth="no"], [$DMCP_LIBS])
> >
> > diff --git a/greeter/Makefile.am b/greeter/Makefile.am
> > index f0a8491..4e0d0a1 100644
> > --- a/greeter/Makefile.am
> > +++ b/greeter/Makefile.am
> > @@ -1,6 +1,10 @@
> > xdmlibdir = $(XDMLIBDIR)
> >
> > +if DYNAMIC_GREETER
> > xdmlib_LTLIBRARIES = libXdmGreet.la
> > +else
> > +noinst_LTLIBRARIES = libXdmGreet.la
> > +endif
> >
> > libXdmGreet_la_SOURCES = \
> > Login.c \
> > @@ -14,7 +18,11 @@ libXdmGreet_la_LIBADD = $(XDMGREET_LIBS)
> > AM_CPPFLAGS = -I$(top_srcdir)/include
> > AM_CFLAGS = $(CWARNFLAGS) $(XDMGREET_CFLAGS) -DGREET_LIB
> >
> > +if DYNAMIC_GREETER
> > libXdmGreet_la_LDFLAGS = -module -avoid-version
> > +else
> > +libXdmGreet_la_LDFLAGS = -static
> > +endif
> >
> > if LINT
> > ALL_LINT_FLAGS=$(LINT_FLAGS) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) \
> > diff --git a/greeter/greet.c b/greeter/greet.c
> > index 82e2c21..9fb6b82 100644
> > --- a/greeter/greet.c
> > +++ b/greeter/greet.c
> > @@ -96,7 +96,7 @@ extern int getdomainname(char *name, size_t len);
> > /*
> > * Function pointers filled in by the initial call ito the library
> > */
> > -
> > +#ifdef GREET_LIB
> > int (*__xdm_PingServer)(struct display *d, Display *alternateDpy) = NULL;
> > void (*__xdm_SessionPingFailed)(struct display *d) = NULL;
> > void (*__xdm_Debug)(const char * fmt, ...) = NULL;
> > @@ -133,6 +133,7 @@ char *(*__xdm_crypt)(CRYPT_ARGS) = NULL;
> > # ifdef USE_PAM
> > pam_handle_t **(*__xdm_thepamhp)(void) = NULL;
> > # endif
> > +#endif
> >
> > #ifdef SECURE_RPC
> > # include <rpc/rpc.h>
> > @@ -426,6 +427,7 @@ greet_user_rtn GreetUser(
> > int i;
> > Arg arglist[2];
> >
> > +#ifdef GREET_LIB
> > /*
> > * These must be set before they are used.
> > */
> > @@ -465,7 +467,7 @@ greet_user_rtn GreetUser(
> > # ifdef USE_PAM
> > __xdm_thepamhp = dlfuncs->_thepamhp;
> > # endif
> > -
> > +#endif
> > *dpy = InitGreet (d);
> > /*
> > * Run the setup script - note this usually will not work when
> > diff --git a/xdm/Makefile.am b/xdm/Makefile.am
> > index 797b5c5..6748ef1 100644
> > --- a/xdm/Makefile.am
> > +++ b/xdm/Makefile.am
> > @@ -24,9 +24,12 @@ bin_PROGRAMS = xdm
> > AM_CPPFLAGS = -I$(top_srcdir)/include
> > AM_CFLAGS = $(CWARNFLAGS) $(XDM_CFLAGS) $(SYSTEMD_DAEMON_CFLAGS)
> >
> > +AM_LDFLAGS = $(XDM_LIBS) $(SYSTEMD_DAEMON_LIBS)
> > # The xdm binary needs to export symbols so that they can be used from
> > # libXdmGreet.so loaded through a dlopen call from session.c
> > -AM_LDFLAGS = $(XDM_LIBS) $(SYSTEMD_DAEMON_LIBS) -export-dynamic
> > +if DYNAMIC_GREETER
> > +AM_LDFLAGS = $(AM_LDFLAGS) -export-dynamic
> > +endif
> >
> > xdm_SOURCES = \
> > access.c \
> > diff --git a/xdm/session.c b/xdm/session.c
> > index 573747d..fe9024d 100644
> > --- a/xdm/session.c
> > +++ b/xdm/session.c
> > @@ -76,10 +76,12 @@ extern int key_setnet(struct key_netstarg *arg);
> > #include <selinux/get_context_list.h>
> > #endif /* USE_SELINUX */
> >
> > +#ifndef GREET_USER_STATIC
> > # include <dlfcn.h>
> > # ifndef RTLD_NOW
> > # define RTLD_NOW 1
> > # endif
> > +#endif
> >
> > #ifdef USE_SYSTEMD_DAEMON
> > #include <systemd/sd-daemon.h>
> > @@ -329,7 +331,9 @@ ManageSession (struct display *d)
> > Display *dpy;
> > greet_user_rtn greet_stat;
> > static GreetUserProc greet_user_proc = NULL;
> > +#ifndef GREET_USER_STATIC
> > void *greet_lib_handle;
> > +#endif
> >
> > Debug ("ManageSession %s\n", d->name);
> > (void)XSetIOErrorHandler(IOErrorHandler);
> > @@ -344,6 +348,9 @@ ManageSession (struct display *d)
> > */
> > LoadXloginResources (d);
> >
> > +#ifdef GREET_USER_STATIC
> > + greet_user_proc = GreetUser;
> > +#else
> > Debug ("ManageSession: loading greeter library %s\n", greeterLib);
> > greet_lib_handle = dlopen(greeterLib, RTLD_NOW);
> > if (greet_lib_handle != NULL)
> > @@ -352,6 +359,8 @@ ManageSession (struct display *d)
> > LogError ("%s while loading %s\n", dlerror(), greeterLib);
> > exit(UNMANAGE_DISPLAY);
> > }
> > +#endif
> > +
> >
> > #ifdef USE_SYSTEMD_DAEMON
> > /* Subsequent notifications will be ignored by systemd
> > @@ -376,9 +385,10 @@ ManageSession (struct display *d)
> > */
> > if (StartClient (&verify, d, &clientPid, greet.name, greet.password)) {
> > Debug ("Client Started\n");
> > -
> > +#ifndef GREET_USER_STATIC
> > /* Save memory; close library */
> > dlclose(greet_lib_handle);
> > +#endif
> >
> > /*
> > * Wait for session to end,
>
>
--
Matthieu Herrb
More information about the xorg-devel
mailing list