date:20151122

Re: [PATCHv3] build: Fix systemd-daemon compile/linker flags

2015-11-22 Thread Peter Hutterer

On Thu, Nov 19, 2015 at 04:04:30PM +0200, Jussi Kukkonen wrote:
> * Dont add systemd to REQUIRED_LIBS (this lead to build failure when
>   libsystemd is available but libsystemd-daemon is not)
> * Only use systemd compile and linker flags where needed (libxtrans)
> * Try libsystemd (>= 210) first so it's used instead of the
>   libsystemd-daemon wrapper that might also exist
> 
> Signed-off-by: Jussi Kukkonen 
> ---
> 
> Let's try once more. Tried to address Peters comments by limiting
> systemd flag use to libos.la. Also fixed the issue Emil pointed out
> by checking only for libsystemd versions that actually provide the
> required api.
> 
> Note that the actual systemd-daemon api use is not in xserver, but
> in the xtrans "library" (at least that's the only one I could find).

unfortunately, this one doesn't work when dtrace is enabled (it works fine
otherwise). libos.la is the only one where appending a library isn't easy
because we convert libos.la to a dtrace object os.O and link that in - and
that makes us lose the dependency. So maybe it's better to go down the
original path with the REQUIRED_LIBS after all, anything else is going to be
messier than a local hack. How about something like this:

diff --git a/configure.ac b/configure.ac
index 14a5bb8..ecb4263 100644
--- a/configure.ac
+++ b/configure.ac
@@ -836,21 +836,21 @@ AC_ARG_WITH([systemd-daemon],
AS_HELP_STRING([--with-systemd-daemon],
[support systemd socket activation (default: auto)]),
[WITH_SYSTEMD_DAEMON=$withval], [WITH_SYSTEMD_DAEMON=auto])
-PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd-daemon],
-  [HAVE_SYSTEMD_DAEMON=yes],
-  [PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd],
- [HAVE_SYSTEMD_DAEMON=yes], 
[HAVE_SYSTEMD_DAEMON=no])])
+PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd >= 210],
+  [HAVE_SYSTEMD_DAEMON="libsystemd"],
+  [PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd-daemon],
+ 
[HAVE_SYSTEMD_DAEMON="libsystemd-daemon"], [HAVE_SYSTEMD_DAEMON=no])])
 if test "x$WITH_SYSTEMD_DAEMON" = xauto; then
WITH_SYSTEMD_DAEMON="$HAVE_SYSTEMD_DAEMON"
 fi
-if test "x$WITH_SYSTEMD_DAEMON" = xyes; then
+if test "x$WITH_SYSTEMD_DAEMON" != xno; then
if test "x$HAVE_SYSTEMD_DAEMON" = xno; then
AC_MSG_ERROR([systemd support requested but no library has been 
found])
fi
AC_DEFINE(HAVE_SYSTEMD_DAEMON, 1, [Define to 1 if libsystemd-daemon is 
available])
-   REQUIRED_LIBS="$REQUIRED_LIBS libsystemd-daemon"
+   REQUIRED_LIBS="$REQUIRED_LIBS $HAVE_SYSTEMD_DAEMON"
 fi
-AM_CONDITIONAL([HAVE_SYSTEMD_DAEMON], [test "x$HAVE_SYSTEMD_DAEMON" = "xyes"])
+AM_CONDITIONAL([HAVE_SYSTEMD_DAEMON], [test "x$HAVE_SYSTEMD_DAEMON" != "xno"])
 
 if test "x$CONFIG_UDEV" = xyes && test "x$CONFIG_HAL" = xyes; then
AC_MSG_ERROR([Hotplugging through both libudev and hal not allowed])

best to rename it to "SYSTEMD_PKG" or so instead of HAVE_SYSTEMD_DAEMON, a
bit more testing and it should be fine?

Cheers,
   Peter

>  configure.ac   | 5 ++---
>  os/Makefile.am | 4 ++--
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 14a5bb8..997b5e5 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -836,9 +836,9 @@ AC_ARG_WITH([systemd-daemon],
>   AS_HELP_STRING([--with-systemd-daemon],
>   [support systemd socket activation (default: auto)]),
>   [WITH_SYSTEMD_DAEMON=$withval], [WITH_SYSTEMD_DAEMON=auto])
> -PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd-daemon],
> +PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd >= 210],
>[HAVE_SYSTEMD_DAEMON=yes],
> -  [PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd],
> +  [PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd-daemon],
>   [HAVE_SYSTEMD_DAEMON=yes], 
> [HAVE_SYSTEMD_DAEMON=no])])
>  if test "x$WITH_SYSTEMD_DAEMON" = xauto; then
>   WITH_SYSTEMD_DAEMON="$HAVE_SYSTEMD_DAEMON"
> @@ -848,7 +848,6 @@ if test "x$WITH_SYSTEMD_DAEMON" = xyes; then
>   AC_MSG_ERROR([systemd support requested but no library has been 
> found])
>   fi
>   AC_DEFINE(HAVE_SYSTEMD_DAEMON, 1, [Define to 1 if libsystemd-daemon is 
> available])
> - REQUIRED_LIBS="$REQUIRED_LIBS libsystemd-daemon"
>  fi
>  AM_CONDITIONAL([HAVE_SYSTEMD_DAEMON], [test "x$HAVE_SYSTEMD_DAEMON" = 
> "xyes"])
>  
> diff --git a/os/Makefile.am b/os/Makefile.am
> index a1bbb4d..c1e2cb5 100644
> --- a/os/Makefile.am
> +++ b/os/Makefile.am
> @@ -1,6 +1,6 @@
>  noinst_LTLIBRARIES = libos.la
>  
> -AM_CFLAGS = $(DIX_CFLAGS) $(SHA1_CFLAGS)
> +AM_CFLAGS = $(DIX_CFLAGS) $(SHA1_CFLAGS) $(SYSTEMD_DAEMON_CFLAGS)
>  
>  SECURERPC_SRCS = rpcauth.c
>  XDMCP_SRCS = xdmcp.c
> @@ -25,7 +25,7 @@ libos_la_SOURCES =  \
>   xstrans.c   \
>   xprintf.c

Re: [PATCHv3] build: Fix systemd-daemon compile/linker flags

2015-11-22 Thread Peter Hutterer

On Mon, Nov 23, 2015 at 12:07:33AM +, Emil Velikov wrote:
> On 22 November 2015 at 23:37, Peter Hutterer  wrote:
> > On Thu, Nov 19, 2015 at 04:04:30PM +0200, Jussi Kukkonen wrote:
> >> * Dont add systemd to REQUIRED_LIBS (this lead to build failure when
> >>   libsystemd is available but libsystemd-daemon is not)
> >> * Only use systemd compile and linker flags where needed (libxtrans)
> >> * Try libsystemd (>= 210) first so it's used instead of the
> >>   libsystemd-daemon wrapper that might also exist
> >>
> >> Signed-off-by: Jussi Kukkonen 
> >> ---
> >>
> >> Let's try once more. Tried to address Peters comments by limiting
> >> systemd flag use to libos.la. Also fixed the issue Emil pointed out
> >> by checking only for libsystemd versions that actually provide the
> >> required api.
> >>
> >> Note that the actual systemd-daemon api use is not in xserver, but
> >> in the xtrans "library" (at least that's the only one I could find).
> >
> > unfortunately, this one doesn't work when dtrace is enabled (it works fine
> > otherwise). libos.la is the only one where appending a library isn't easy
> > because we convert libos.la to a dtrace object os.O and link that in - and
> > that makes us lose the dependency. So maybe it's better to go down the
> > original path with the REQUIRED_LIBS after all, anything else is going to be
> > messier than a local hack. How about something like this:
> >
> > diff --git a/configure.ac b/configure.ac
> > index 14a5bb8..ecb4263 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -836,21 +836,21 @@ AC_ARG_WITH([systemd-daemon],
> > AS_HELP_STRING([--with-systemd-daemon],
> > [support systemd socket activation (default: auto)]),
> > [WITH_SYSTEMD_DAEMON=$withval], [WITH_SYSTEMD_DAEMON=auto])
> > -PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd-daemon],
> > -  [HAVE_SYSTEMD_DAEMON=yes],
> > -  [PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd],
> > - [HAVE_SYSTEMD_DAEMON=yes], 
> > [HAVE_SYSTEMD_DAEMON=no])])
> > +PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd >= 210],
> > +  [HAVE_SYSTEMD_DAEMON="libsystemd"],
> > +  [PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd-daemon],
> > + 
> > [HAVE_SYSTEMD_DAEMON="libsystemd-daemon"], [HAVE_SYSTEMD_DAEMON=no])])
> >  if test "x$WITH_SYSTEMD_DAEMON" = xauto; then
> > WITH_SYSTEMD_DAEMON="$HAVE_SYSTEMD_DAEMON"
> >  fi
> > -if test "x$WITH_SYSTEMD_DAEMON" = xyes; then
> > +if test "x$WITH_SYSTEMD_DAEMON" != xno; then
> > if test "x$HAVE_SYSTEMD_DAEMON" = xno; then
> > AC_MSG_ERROR([systemd support requested but no library has 
> > been found])
> > fi
> > AC_DEFINE(HAVE_SYSTEMD_DAEMON, 1, [Define to 1 if libsystemd-daemon 
> > is available])
> Nitpick: Use the correct name in above comment ?

sounds good

> 
> > -   REQUIRED_LIBS="$REQUIRED_LIBS libsystemd-daemon"
> > +   REQUIRED_LIBS="$REQUIRED_LIBS $HAVE_SYSTEMD_DAEMON"
> >  fi
> > -AM_CONDITIONAL([HAVE_SYSTEMD_DAEMON], [test "x$HAVE_SYSTEMD_DAEMON" = 
> > "xyes"])
> > +AM_CONDITIONAL([HAVE_SYSTEMD_DAEMON], [test "x$HAVE_SYSTEMD_DAEMON" != 
> > "xno"])
> >
> >  if test "x$CONFIG_UDEV" = xyes && test "x$CONFIG_HAL" = xyes; then
> > AC_MSG_ERROR([Hotplugging through both libudev and hal not allowed])
> >
> > best to rename it to "SYSTEMD_PKG" or so instead of HAVE_SYSTEMD_DAEMON, a
> > bit more testing and it should be fine?
> >
> Why are we using a single variable for "have" and "package name"
> (LIBSYSTEMD_DAEMON or alike in this case) ? Won't things be
> clearer/more consistent if they are kept separate ?

I think we can just use one variable as long as it's not actually called
HAVE_FOO and it may even be clearer than setting two variables. But give
either a try, this was just a quick hack to check if this works, I'll leave
the fine detail work to you ;)

Cheers,
   Peter
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH v3] xwayland: Do not set root clip when rootless

2015-11-22 Thread Olivier Fourdan

Otherwise the server may try to draw onto the root window when closing
down, but when running rootless the root window has no storage thus
causing a memory corruption.

Thanks to Adam Jackson  for helping tracking this down!

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93045
Signed-off-by: Olivier Fourdan 
---
v2: SetRootClip() only if not rootless in update_screen_size()
v3: fix indentation

 hw/xwayland/xwayland-glamor.c | 4 +++-
 hw/xwayland/xwayland-output.c | 6 --
 hw/xwayland/xwayland-shm.c| 4 +++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index ebaf05a..c357217 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -233,9 +233,11 @@ xwl_glamor_create_screen_resources(ScreenPtr screen)
 if (!ret)
 return ret;
 
-if (xwl_screen->rootless)
+if (xwl_screen->rootless) {
 screen->devPrivate =
 fbCreatePixmap(screen, 0, 0, screen->rootDepth, 0);
+SetRootClip(screen, FALSE);
+}
 else {
 screen->devPrivate =
 xwl_glamor_create_pixmap(screen, screen->width, screen->height,
diff --git a/hw/xwayland/xwayland-output.c b/hw/xwayland/xwayland-output.c
index 5ef444d..2a180f2 100644
--- a/hw/xwayland/xwayland-output.c
+++ b/hw/xwayland/xwayland-output.c
@@ -164,7 +164,7 @@ update_screen_size(struct xwl_output *xwl_output, int 
width, int height)
 struct xwl_screen *xwl_screen = xwl_output->xwl_screen;
 double mmpd;
 
-if (xwl_screen->screen->root)
+if (!xwl_screen->rootless)
 SetRootClip(xwl_screen->screen, FALSE);
 
 xwl_screen->width = width;
@@ -184,11 +184,13 @@ update_screen_size(struct xwl_output *xwl_output, int 
width, int height)
 if (xwl_screen->screen->root) {
 xwl_screen->screen->root->drawable.width = width;
 xwl_screen->screen->root->drawable.height = height;
-SetRootClip(xwl_screen->screen, TRUE);
 RRScreenSizeNotify(xwl_screen->screen);
 }
 
 update_desktop_dimensions();
+
+if (!xwl_screen->rootless)
+SetRootClip(xwl_screen->screen, TRUE);
 }
 
 static void
diff --git a/hw/xwayland/xwayland-shm.c b/hw/xwayland/xwayland-shm.c
index 1022c0d..7072be4 100644
--- a/hw/xwayland/xwayland-shm.c
+++ b/hw/xwayland/xwayland-shm.c
@@ -279,9 +279,11 @@ xwl_shm_create_screen_resources(ScreenPtr screen)
 if (!ret)
 return ret;
 
-if (xwl_screen->rootless)
+if (xwl_screen->rootless) {
 screen->devPrivate =
 fbCreatePixmap(screen, 0, 0, screen->rootDepth, 0);
+SetRootClip(screen, FALSE);
+}
 else
 screen->devPrivate =
 xwl_shm_create_pixmap(screen, screen->width, screen->height,
-- 
2.5.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCHv3] build: Fix systemd-daemon compile/linker flags

2015-11-22 Thread Emil Velikov

On 22 November 2015 at 23:37, Peter Hutterer  wrote:
> On Thu, Nov 19, 2015 at 04:04:30PM +0200, Jussi Kukkonen wrote:
>> * Dont add systemd to REQUIRED_LIBS (this lead to build failure when
>>   libsystemd is available but libsystemd-daemon is not)
>> * Only use systemd compile and linker flags where needed (libxtrans)
>> * Try libsystemd (>= 210) first so it's used instead of the
>>   libsystemd-daemon wrapper that might also exist
>>
>> Signed-off-by: Jussi Kukkonen 
>> ---
>>
>> Let's try once more. Tried to address Peters comments by limiting
>> systemd flag use to libos.la. Also fixed the issue Emil pointed out
>> by checking only for libsystemd versions that actually provide the
>> required api.
>>
>> Note that the actual systemd-daemon api use is not in xserver, but
>> in the xtrans "library" (at least that's the only one I could find).
>
> unfortunately, this one doesn't work when dtrace is enabled (it works fine
> otherwise). libos.la is the only one where appending a library isn't easy
> because we convert libos.la to a dtrace object os.O and link that in - and
> that makes us lose the dependency. So maybe it's better to go down the
> original path with the REQUIRED_LIBS after all, anything else is going to be
> messier than a local hack. How about something like this:
>
> diff --git a/configure.ac b/configure.ac
> index 14a5bb8..ecb4263 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -836,21 +836,21 @@ AC_ARG_WITH([systemd-daemon],
> AS_HELP_STRING([--with-systemd-daemon],
> [support systemd socket activation (default: auto)]),
> [WITH_SYSTEMD_DAEMON=$withval], [WITH_SYSTEMD_DAEMON=auto])
> -PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd-daemon],
> -  [HAVE_SYSTEMD_DAEMON=yes],
> -  [PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd],
> - [HAVE_SYSTEMD_DAEMON=yes], 
> [HAVE_SYSTEMD_DAEMON=no])])
> +PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd >= 210],
> +  [HAVE_SYSTEMD_DAEMON="libsystemd"],
> +  [PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd-daemon],
> + 
> [HAVE_SYSTEMD_DAEMON="libsystemd-daemon"], [HAVE_SYSTEMD_DAEMON=no])])
>  if test "x$WITH_SYSTEMD_DAEMON" = xauto; then
> WITH_SYSTEMD_DAEMON="$HAVE_SYSTEMD_DAEMON"
>  fi
> -if test "x$WITH_SYSTEMD_DAEMON" = xyes; then
> +if test "x$WITH_SYSTEMD_DAEMON" != xno; then
> if test "x$HAVE_SYSTEMD_DAEMON" = xno; then
> AC_MSG_ERROR([systemd support requested but no library has 
> been found])
> fi
> AC_DEFINE(HAVE_SYSTEMD_DAEMON, 1, [Define to 1 if libsystemd-daemon 
> is available])
Nitpick: Use the correct name in above comment ?

> -   REQUIRED_LIBS="$REQUIRED_LIBS libsystemd-daemon"
> +   REQUIRED_LIBS="$REQUIRED_LIBS $HAVE_SYSTEMD_DAEMON"
>  fi
> -AM_CONDITIONAL([HAVE_SYSTEMD_DAEMON], [test "x$HAVE_SYSTEMD_DAEMON" = 
> "xyes"])
> +AM_CONDITIONAL([HAVE_SYSTEMD_DAEMON], [test "x$HAVE_SYSTEMD_DAEMON" != 
> "xno"])
>
>  if test "x$CONFIG_UDEV" = xyes && test "x$CONFIG_HAL" = xyes; then
> AC_MSG_ERROR([Hotplugging through both libudev and hal not allowed])
>
> best to rename it to "SYSTEMD_PKG" or so instead of HAVE_SYSTEMD_DAEMON, a
> bit more testing and it should be fine?
>
Why are we using a single variable for "have" and "package name"
(LIBSYSTEMD_DAEMON or alike in this case) ? Won't things be
clearer/more consistent if they are kept separate ?

Thanks
Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver] xfree86: fix minor memory leak

2015-11-22 Thread Peter Hutterer

xf86*StrOption returns a strdup

Signed-off-by: Peter Hutterer 
---
 hw/xfree86/common/xf86Xinput.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
index a5b0568..c56a2b9 100644
--- a/hw/xfree86/common/xf86Xinput.c
+++ b/hw/xfree86/common/xf86Xinput.c
@@ -843,7 +843,7 @@ xf86NewInputDevice(InputInfoPtr pInfo, DeviceIntPtr *pdev, 
BOOL enable)
 DeviceIntPtr dev = NULL;
 Bool paused;
 int rval;
-const char *path;
+char *path = NULL;
 
 /* Memory leak for every attached device if we don't
  * test if the module is already loaded first */
@@ -873,6 +873,7 @@ xf86NewInputDevice(InputInfoPtr pInfo, DeviceIntPtr *pdev, 
BOOL enable)
 new_input_devices[new_input_devices_count] = pInfo;
 new_input_devices_count++;
 systemd_logind_release_fd(pInfo->major, pInfo->minor, fd);
+free(path);
 return BadMatch;
 }
 pInfo->fd = fd;
@@ -881,6 +882,8 @@ xf86NewInputDevice(InputInfoPtr pInfo, DeviceIntPtr *pdev, 
BOOL enable)
 }
 }
 
+free(path);
+
 xf86Msg(X_INFO, "Using input driver '%s' for '%s'\n", drv->driverName,
 pInfo->name);
 
-- 
2.5.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCHv3] build: Fix systemd-daemon compile/linker flags

Re: [PATCHv3] build: Fix systemd-daemon compile/linker flags

[PATCH v3] xwayland: Do not set root clip when rootless

Re: [PATCHv3] build: Fix systemd-daemon compile/linker flags

[PATCH xserver] xfree86: fix minor memory leak

5 matches

Site Navigation

Mail list logo

Footer information