Re: [PATCH app/xcursorgen v3] Update README for gitlab migration

[Ray Strode]

hi,

On Tue, Nov 13, 2018, 7:57 PM Alan Coopersmith  Anyone have a preference?   Shipping README.md seems easier - will it cause
> problems for any distros or packagers?
>
i think README.md is the right way to go.  presumably any packaging
challenges have been ironed out by now considering how widespread
gitlab/github is now.

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

Re: [PATCH app/xcursorgen] Update README for gitlab migration

[Ray Strode]

hi,

On Mon, Oct 1, 2018, 1:11 PM Alan Coopersmith 
wrote:

> Does anyone have feedback on changing our README's like this?

it should probably be renamed README.md so it shows up in the gitlab view
better.

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

Re: Xorg-1.20.1 crashes when using glamor on top of llvmpipe

[Ray Strode]

hi,

what version of mesa?

might be


https://cgit.freedesktop.org/mesa/mesa/commit/?id=9baff597ce021f7691187b0d1d1bbc16d07b13e1

Ray

On Thu, Aug 30, 2018, 5:00 PM Hans de Goede  wrote:

> Hi,
>
> On 30-08-18 22:48, Hans de Goede wrote:
> > HI all,
> >
> > I've been debugging some strange crashes with Xorg-1.20.1 inside
> > a virtualbox guest and I can use some help with this.
> >
> > At first Xorg completely failed to start, running it under
> > gdbserver showed a backtrace pointing to a lazy symbol lookup
> > failure triggered by:
> >
> > drmmode_display.c:905:
> >  return gbm_bo_get_stride(bo->gbm);
> >
> > Which is part of:
> >
> >
> > uint32_t
> > drmmode_bo_get_pitch(drmmode_bo *bo)
> > {
> > #ifdef GLAMOR_HAS_GBM
> >  if (bo->gbm)
> >  return gbm_bo_get_stride(bo->gbm);
> > #endif
> >
> >  return bo->dumb->pitch;
> > }
> >
> > Strange enough a LD_PRELOAD of libgbm does not
> > fix this and libgbm already gets dragged in by
> > libglamor_egl.so so this should not be a problem.
> >
> > Still I tried this change:
> >
> > --- a/hw/xfree86/drivers/modesetting/Makefile.am
> > +++ b/hw/xfree86/drivers/modesetting/Makefile.am
> > @@ -39,7 +39,7 @@ AM_CPPFLAGS = \
> >
> >   modesetting_drv_la_LTLIBRARIES = modesetting_drv.la
> >   modesetting_drv_la_LDFLAGS = -module -avoid-version
> > -modesetting_drv_la_LIBADD = $(UDEV_LIBS) $(DRM_LIBS)
> > +modesetting_drv_la_LIBADD = $(UDEV_LIBS) $(DRM_LIBS) $(GBM_LIBS)
> >   modesetting_drv_ladir = @moduledir@/drivers
> >
> >   modesetting_drv_la_SOURCES = \
> >
> > And now Xorg will start, still very weird since the
> > exact same Xorg binaries work fine on Intel integrated
> > gfx where the gbm_bo_get_stride() call also happens ...
> >
> >
> > So with this "fix" it starts, but it crashes as soon as I resize the
> > vm-window and thus the screen gets resized:
> >
> > bt
> > #0  OsSigHandler (signo=11, sip=0x7ffc160ad2f0, unused=0x7ffc160ad1c0)
> >  at osinit.c:114
> > #1  
> > #2  miModifyPixmapHeader (pPixmap=0x28c4460, width=1920, height=992,
> depth=-1,
> >  bitsPerPixel=-1, devKind=7680, pPixData=0x0) at miscrinit.c:64
> > #3  0x7fdc13d64471 in drmmode_xf86crtc_resize (scrn=0x21751f0,
> width=1920,
> >  height=992) at drmmode_display.c:3166
> > #4  0x004bb9d8 in xf86RandR12ScreenSetSize (pScreen=0x23dc9b0,
> >  width=1920, height=992, mmWidth=508, mmHeight=262) at
> xf86RandR12.c:698
> > #5  0x005092f0 in ProcRRSetScreenSize (client=0x29c6af0)
> >  at rrscreen.c:289
> > #6  0x0043fcee in Dispatch () at dispatch.c:478
> >
> > And miscrinit.c:64 is hte "{" of:
> >
> > Bool
> > miModifyPixmapHeader(PixmapPtr pPixmap, int width, int height, int depth,
> >   int bitsPerPixel, int devKind, void *pPixData)
> > {
> >  if (!pPixmap)
> >  return FALSE;
> >
> > Which is a strange place to crash. Even more weird after adding a
> > breakpoint a bit before drmmode_display.c:3166, I get a segfault
> > while stepping through earlier lines, pointing at SmartScheduleTimer()
> > and specifically again at the opening "{" as if something is wrong with
> > the stack and the stack cannot handle function calls being stacked one
> > level deeper.
> >
> > This makes me wonder if this is a stack depth/overflow issue, does the
> > xserver have code somewhere to limit its stacksize and could we be
> > hitting that ?
> >
> > Or maybe a bad interaction with gcc-s stack protection?
> >
> > This feels as if we are hitting a guard page at the end of the stack
> > here?
>
> One important thing which I only put in the subject, this happens
> when using glmamor with llvmpipe, something which is new in 1.20,
> older xservers never used glamor on llvmpipe. Things work fine
> if I disable glamor in a xorg.conf snippet.
>
> Arguably we should disable glamor when running on llvmpipe because
> of performance reasons, still these crashes should not happen.
>
> Regards,
>
> Hans
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 05/10] test: Add a little xinit-like program for starting servers for testing.

[Ray Strode]

Hey cool,

On Friday, September 23, 2016, Eric Anholt  wrote:
>
> The normal xinit is racy because it doesn't use -displayfd.  This
> implements the bare minimum for testing purposes, using -displayfd to
> sequence starting the client, and avoids adding yet another dependency
> to the server.

Any chance you want to work on the xinit changes proposed here?
https://lists.x.org/archives/xorg-devel/2015-March/045948.html

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

Re: [PATCH xserver 0/2] RFC: Sync key repeat with Wayland compositor

[Ray Strode]

Hi,

> This still suffers from your priority-starvation-bug. If a simple
> ping/pong protocol barrier doesn't solve the issue, why would the
> timer? You'd have to put the timer on lowest priority to make that
> work, but then again you can do the same for wl_display_sync.
Yup, indeed, you're right. I guess we just need to fix the compositor
here.

> Btw., if your compositor stalls for a bit more, you end up with an
> evdev-queue overrun and have more trouble than you can solve. Is it
> really worth solving that single race, while there're still several
> known other issues that happen on stalling compositors?
My concern is that we don't end up with spurious repeat events.
I don't want a relatively benign compositor stall (which is a bug
and should be fixed!) leading to 15 mails getting deleted instead
of 1 mail getting deleted when the user hits delete once.

> Why not rather put keyboard handling on high-priority?
> It is low-throughput, anyway.
fair.

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

Re: [PATCH xserver 0/2] RFC: Sync key repeat with Wayland compositor

[Ray Strode]

Hi,

On Tue, Mar 8, 2016 at 7:09 AM, Daniel Stone  wrote:
>> If one justification for server-side repeat is that if the compositor
>> is hosed and the user cannot see how many characters have been
>> repeated, then you could as well solve that in the client too, by
>> throttling your repeats to frame callbacks, but only when it makes sense
>> and undoing repeats is not feasible.
>
> This is what has just been implemented for GTK+.
No, the first draft of the patch was implemented that way for GTK+, but
Jonas pointed out a problem with the approach.  Not all key events lead
to screen updates.  For instance, some users hold down backspace for
a few seconds if they bungle their password at a terminal.  The expectation
is that holding down backspace in that case, should do the same thing
as hitting ctrl-u I guess (clear everything).

The current approach uses wl_display_sync to throttle, but that has a
problem too: clutter processes hardware input events at a lower priority
than mutter processes wayland events
( G_PRIORITY_HIGH_IDLE + 50 (150) vs G_PRIORITY_DEFAULT + 1 (1),
 lower is more urgent), so it's possible the sync could reply before a user
release event is processed.  It might be possible to fix that in
mutter/clutter, but might be tricky.

Anyway, clearly, the compositor stalling is bad, but if it happens we
shouldn't send a stream of the last character the user typed into the
application. That turns a momentary lock up into unpredictable
interaction with the application.  that's awful.

I guess if we don't do server repeat events, an alternative would be to
add a server timer protocol.  a client could request to be notified when
n milliseconds has passed.  Might be useful for other use cases too.

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

Re: [PATCH] Fix NO_LOCAL_CLIENT_CRED build

[Ray Strode]

Hi,

On Mon, Jul 6, 2015 at 6:08 PM, Keith Packard kei...@keithp.com wrote:
 I liked your first version a lot better; looks a lot simpler. An
 autoconf test might make sense if there was some reason to override it?

To be clear, I primarily gave feedback because touched it last.  I
think it makes more sense to consolidate platform checks in
configure.ac (overridable or static) because the configure script is
what does the lion's share of platform checks.  Still, it's no doubt a
style question, and I'll defer on style matters to Keith/those more
intimately involved in the project than me.

--Ray
___
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: [PATCH] Fix NO_LOCAL_CLIENT_CRED build

[Ray Strode]

Hi,


On Thu, Jul 2, 2015 at 12:40 PM, Jon TURNEY jon.tur...@dronecode.org.uk wrote:
 Makes sense. Revised patch attached.
LGTM

--Ray
___
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: [PATCH] Fix NO_LOCAL_CLIENT_CRED build

[Ray Strode]

Hi,

 Yes, I think so.  Revised patch attached.
 
 I've tested this a few ways and it seems to be working correctly, but
 it's hard for me to be sure that this is doing the correct thing on all
 targets.

Thanks for working on this! Seems right to me.  My only
comment is I think have_so_peercred could be prefixed with 
xorg_cv_sys_ / AC_CACHE_CHECK could be used to interface with
configure's caching mechanism, but I don't think that really matters.

--Ray
___
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: [PATCH] Fix NO_LOCAL_CLIENT_CRED build

[Ray Strode]

Hi,

 This is a build fix for MinGW
... 
 Move the check if NO_LOCAL_CLIENT_CRED should be defined to before it's first
 use.
Well, Alan wondered if anyone is actually using NO_LOCAL_CLIENT_CRED, now we 
know!

Patch doesn't look wrong to me, but I wonder if maybe it should get put in 
configure.ac?
I think that's a more typical place for platform specific definitions.

--Ray
___
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: [PATCH 1/2] systemd-logind: filter out non-signal messages from message filter

[Ray Strode]

Hi,

 Ray, you may want to reduce the number of context lines a bit next time
 you post patches.

Nope, definitely don't. my default config produces patches with a lot
of context intentionally. I want driveby readers who aren't motivated
enough to go to the source tree to still be able to do a cursory
review from just the patches themselves.

Clearly it doesn't help with full reviews, but by making life easier
for someone casually trawling email or bugzilla, I get more eyes for
free.

--Ray
___
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: [PATCH 1/2] systemd-logind: filter out non-signal messages from message filter

[Ray Strode]

Hi,

On Fri, Apr 17, 2015 at 10:43 AM, Hans de Goede
 You may be making life easier for casual reviewers, but you are making
life
 harder for the people actually merging your patches as the chances of a
 conflict increase enormously with such a large context.

Well, we all know how to deal with small conflicts pretty effortlessly I think.
And, if there are non-trivial changes to the surrounding code, then
it applies isn't really a good approximation for patch is still
correct anyway.

I agree it's a trade off, but I think more context is usually the
better way to go.

You're free to disagree, of course, and that's fine.

--Ray
___
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 2/2] systemd-logind: don't second guess D-Bus default timeout

[Ray Strode]

At the moment, the X server uses a non-default timeout for D-Bus
messages to systemd-logind. The only timeouts normally used with
D-Bus are:

1) Infinite
2) Default

Anything else is just as arbitrary as Default, and so rarely makes
sense to use instead of Default.

Put another way, there's little reason to be fault tolerant against
a local root running daemon (logind), that in some configurations, the
X server already depends on for proper functionality.

This commit changes systemd-logind to just use the default timeouts.

Downstream-bug: https://bugzilla.redhat.com/show_bug.cgi?id=1209347
Signed-off-by: Ray Strode rstr...@redhat.com
---
 hw/xfree86/os-support/linux/systemd-logind.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/hw/xfree86/os-support/linux/systemd-logind.c 
b/hw/xfree86/os-support/linux/systemd-logind.c
index 57c87c0..4ad41a3 100644
--- a/hw/xfree86/os-support/linux/systemd-logind.c
+++ b/hw/xfree86/os-support/linux/systemd-logind.c
@@ -13,62 +13,60 @@
  * Software.
  *
  * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
  * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
  * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
  * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
  * DEALINGS IN THE SOFTWARE.
  *
  * Author: Hans de Goede hdego...@redhat.com
  */
 
 #ifdef HAVE_XORG_CONFIG_H
 #include xorg-config.h
 #endif
 
 #include dbus/dbus.h
 #include string.h
 #include sys/types.h
 #include unistd.h
 
 #include os.h
 #include dbus-core.h
 #include xf86.h
 #include xf86platformBus.h
 #include xf86Xinput.h
 
 #include systemd-logind.h
 
-#define DBUS_TIMEOUT 500 /* Wait max 0.5 seconds */
-
 struct systemd_logind_info {
 DBusConnection *conn;
 char *session;
 Bool active;
 Bool vt_active;
 };
 
 static struct systemd_logind_info logind_info;
 
 static InputInfoPtr
 systemd_logind_find_info_ptr_by_devnum(InputInfoPtr start,
int major, int minor)
 {
 InputInfoPtr pInfo;
 
 for (pInfo = start; pInfo; pInfo = pInfo-next)
 if (pInfo-major == major  pInfo-minor == minor 
 (pInfo-flags  XI86_SERVER_FD))
 return pInfo;
 
 return NULL;
 }
 
 static void
 systemd_logind_set_input_fd_for_all_devs(int major, int minor, int fd,
  Bool enable)
 {
 InputInfoPtr pInfo;
 
 pInfo = systemd_logind_find_info_ptr_by_devnum(xf86InputDevs, major, 
minor);
@@ -103,61 +101,61 @@ systemd_logind_take_fd(int _major, int _minor, const char 
*path,
 if (strstr(path, mouse))
 return -1;
 
 /* Check if we already have an InputInfo entry with this major, minor
  * (shared device-nodes happen ie with Wacom tablets). */
 pInfo = systemd_logind_find_info_ptr_by_devnum(xf86InputDevs, major, 
minor);
 if (pInfo) {
 LogMessage(X_INFO, systemd-logind: returning pre-existing fd for %s 
%u:%u\n,
path, major, minor);
 *paused_ret = FALSE;
 return pInfo-fd;
 }
 
 dbus_error_init(error);
 
 msg = dbus_message_new_method_call(org.freedesktop.login1, info-session,
 org.freedesktop.login1.Session, TakeDevice);
 if (!msg) {
 LogMessage(X_ERROR, systemd-logind: out of memory\n);
 goto cleanup;
 }
 
 if (!dbus_message_append_args(msg, DBUS_TYPE_UINT32, major,
DBUS_TYPE_UINT32, minor,
DBUS_TYPE_INVALID)) {
 LogMessage(X_ERROR, systemd-logind: out of memory\n);
 goto cleanup;
 }
 
 reply = dbus_connection_send_with_reply_and_block(info-conn, msg,
-  DBUS_TIMEOUT, error);
+  
DBUS_TIMEOUT_USE_DEFAULT, error);
 if (!reply) {
 LogMessage(X_ERROR, systemd-logind: failed to take device %s: %s\n,
path, error.message);
 goto cleanup;
 }
 
 if (!dbus_message_get_args(reply, error,
DBUS_TYPE_UNIX_FD, fd,
DBUS_TYPE_BOOLEAN, paused,
DBUS_TYPE_INVALID)) {
 LogMessage(X_ERROR, systemd-logind: TakeDevice %s: %s\n,
path, error.message);
 goto cleanup;
 }
 
 *paused_ret = paused;
 
 LogMessage(X_INFO, systemd-logind: got fd for %s %u:%u fd %d paused %d\n,
path, major, minor, fd, paused);
 
 cleanup:
 if (msg)
 dbus_message_unref(msg);
 if (reply)
 dbus_message_unref(reply);
 dbus_error_free(error);
 
 return fd;
 }
 
@@ -180,61 +178,61 @@ systemd_logind_release_fd(int

[PATCH 1/2] systemd-logind: filter out non-signal messages from message filter

[Ray Strode]

It's possible to receive a message reply in the message filter if a
previous message call timed out locally before the reply arrived.

The message_filter function only handles signals, at the moment, and
does not properly handle message replies.

This commit changes the message_filter function to filter out all
non-signal messages, including spurious message replies.

Downstream-bug: https://bugzilla.redhat.com/show_bug.cgi?id=1209347
Signed-off-by: Ray Strode rstr...@redhat.com
---
 hw/xfree86/os-support/linux/systemd-logind.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/xfree86/os-support/linux/systemd-logind.c 
b/hw/xfree86/os-support/linux/systemd-logind.c
index 49758f4..57c87c0 100644
--- a/hw/xfree86/os-support/linux/systemd-logind.c
+++ b/hw/xfree86/os-support/linux/systemd-logind.c
@@ -286,60 +286,63 @@ systemd_logind_ack_pause(struct systemd_logind_info *info,
DBUS_TYPE_INVALID)) {
 LogMessage(X_ERROR, systemd-logind: out of memory\n);
 goto cleanup;
 }
 
 reply = dbus_connection_send_with_reply_and_block(info-conn, msg,
   DBUS_TIMEOUT, error);
 if (!reply)
 LogMessage(X_ERROR, systemd-logind: failed to ack pause: %s\n,
error.message);
 
 cleanup:
 if (msg)
 dbus_message_unref(msg);
 if (reply)
 dbus_message_unref(reply);
 dbus_error_free(error);
 }
 
 static DBusHandlerResult
 message_filter(DBusConnection * connection, DBusMessage * message, void *data)
 {
 struct systemd_logind_info *info = data;
 struct xf86_platform_device *pdev = NULL;
 InputInfoPtr pInfo = NULL;
 int ack = 0, pause = 0, fd = -1;
 DBusError error;
 dbus_int32_t major, minor;
 char *pause_str;
 
+if (dbus_message_get_type (message) != DBUS_MESSAGE_TYPE_SIGNAL)
+return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+
 dbus_error_init(error);
 
 if (dbus_message_is_signal(message,
org.freedesktop.DBus, NameOwnerChanged)) {
 char *name, *old_owner, *new_owner;
 
 dbus_message_get_args(message, error,
   DBUS_TYPE_STRING, name,
   DBUS_TYPE_STRING, old_owner,
   DBUS_TYPE_STRING, new_owner, DBUS_TYPE_INVALID);
 if (dbus_error_is_set(error)) {
 LogMessage(X_ERROR, systemd-logind: NameOwnerChanged: %s\n,
error.message);
 dbus_error_free(error);
 return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
 if (name  strcmp(name, org.freedesktop.login1) == 0)
 FatalError(systemd-logind disappeared (stopped/restarted?)\n);
 
 return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
 if (strcmp(dbus_message_get_path(message), info-session) != 0)
 return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
 if (dbus_message_is_signal(message, org.freedesktop.login1.Session,
PauseDevice)) {
 if (!dbus_message_get_args(message, error,
DBUS_TYPE_UINT32, major,
-- 
2.3.5

___
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: [PATCH xinit 2/2] startx: Make startx auto display select work with per user /tmp dirs

[Ray Strode]

Hi,

 Now, I've tried to avoid anything xauth-related, but from the little I know:
 to support displayfd in startx you'd have to communicate back to startx
 about the $DISPLAY and do the xauth dance before continuing with the xinit
 initial client connection. AFAICT, that's the tricky bit about -displayfd
 support in startx. Does that make sense or am I way off here?
Sending $DISPLAY back to startx isn't actually an option since, $DISPLAY comes
from the X server, and the auth file has to be prepared before
starting the X server.
If you start the X server without the auth file then the X server will
get started wide
open to anyone on the host. Sure you could lock it down at that point,
but then there's
a race where anyone could open the display and snoop from then on.

The two ideas I proposed in my other mail are the only secure ways I
can think to
go forward. Either:

1) Have xinit write out the auth file itself (with the $DISPLAY wildcard)
or
2) Fix /usr/bin/xauth to allow adding a $DISPLAY wildcard and change startx to
use the wildcard.

--Ray
___
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: [PATCH xinit 2/2] startx: Make startx auto display select work with per user /tmp dirs

[Ray Strode]

Hi,

 You're right I had forgotten that the xauth file needs to contain the
 displaynr, so there is no easy fix here I'm afraid.
That's not true actually. The xauth file can have a display wildcard, but
the xauth command doesn't support generating xauth files with display
wildcards.

 I think that if we want to fix this we should add a -autodisplayconfig
 config to xinit which takes care of handling both -displayfd and of
 generating and passing in an xauth file.
Yea, exactly!  I would go a step further and just always do what the
-autodisplayconfig line does, implicitly. Of course, if the caller passes
in a -auth line and $DISPLAY, use the ones passed in, but always do
-displayfd no matter what, since -displayfd works fine with a passed
in $DISPLAY and is still useful for getting ready notification without
having to use unix signal handlers.

--Ray
___
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: [PATCH xinit 1/2] startx: Fix startx picking an already used display number when -nolock is used

[Ray Strode]

Hi,

On Fri, Mar 20, 2015 at 1:01 PM, Jasper St. Pierre
jstpie...@mecheye.net wrote:
 ... Why does displayfd imply nolock?
It doesn't really matter why it implies, nolock, I guess. The issue at
hand is that it
does (and always has) implied nolock.  You can't fix that without
breaking things that
rely on it being nolock (like mutter).

 I consider the .X0-lock files an API that we shouldn't break.
Boat's already sailed on this one.  X server already supports a number
of modes that
don't use lock files, and you can't change that without breaking
things in the wild.

Anyway, that aside, they are a bad idea. They create a trivial denial of service
opportunity for users with /tmp access but not local access, they
clutter up /tmp,
and the role they serve is already covered by the more modern
-displayfd construct.

--Ray
___
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: [PATCH xinit 2/2] startx: Make startx auto display select work with per user /tmp dirs

[Ray Strode]

Hi,

On Fri, Mar 20, 2015 at 10:02 AM, Hans de Goede hdego...@redhat.com wrote:
 If a separate /tmp per user is used the existing auto display select code
 does not work, add an extra check for the unix socket for the display number
 existing in /proc/net/unix (linux only).

This patch and the previous patch make sense to me near term.  It
fixes some corner
cases without regressing any others.  I do think a better medium-term
solution would
be for xinit to start using -displayfd and startx to drop its try to
figure out a display
number heuristics.  There's just no reason programs that start an X
server should be
trawling around in /tmp and/or binding to sockets up front (except for
hysterical raisins)

--Ray
___
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: Using systemd-logind Session.TakeControl() from Xorg, input needed

[Ray Strode]

Hi,

- Original Message -
 Ray Strode also said: You can't shut down an X server unless
 it's in the foreground.. I can remember having done that
 without problems just yesterday when working on systemd socket
 activation for the xserver. I've just tried and I can happily
 kill (normal kill not -9) Xorg :1 running on vt2 while I'm
 inside a terminal on Xorg :0 running on vt1:
I don't think that works in all cases, but I can't be more specific,
because it's vague in my head so others would have to chime in.

Another issue, is X jumps back to the VT it was started on when it exits,
but I think we can work around that behavior by making sure the X server
acts as if it was started with -novtswitch or so.

I started working on wayland integration a couple of months ago, but put it
to the side when I got side tracked by other projects. That's here:

https://git.gnome.org/browse/gdm/log/?h=wip/wayland

On that branch (which isn't ready for prime time yet) you can put:

X-GDM-NeedsVT=true

in the xsession file for the session then gdm will allocate
a VT and jump to that VT before running the session.  If the X server is
activated implicitly by the session trying to connect to the socket in 
XDG_RUNTIME_DIR,
then things should just work without GDM managing the display at all.  GDM 
won't try
to reuse the greeter's X server or start a new X server explicitly if 
X-GDM-NeedsVT=true.

The branch is still in progress and I don't exactly remember what state it's 
in/what bugs
it has, but I'm planning on picking it back up next week. after I finish up this
RHEL work, I'll play with the idea then and report back (unless you get to it 
first).

--Ray
___
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: Using systemd-logind Session.TakeControl() from Xorg, input needed

[Ray Strode]

Hi,

- Original Message -
 No. gdm uses a single Xserver for the login-screen and for the
 session. But once you log in multiple times, I think it starts a new
 Xserver for each. But I am not that familiar with gdm..
 
  * Can gdm pass in the session id, or the pid of the session leader
to X when starting X ?
 
 Just use sd_pid_get_session(). It's in systemd/sd-login.h. It returns
 the session ID of a given process. Don't pass session-ids around.
We don't actually run the X server in a session at the moment.

 Start one Xserver for each session. Yes, blending and other transition
 effects will get very hard to implement then, but still better than
 sharing an xserver between sessions..
I think it might be better to wait on that until we have wayland and a system 
compositor.

 Why undesirable? That's the way to go.
1) It will cause flicker
2) It will mean there's always an X server with a login screen running in the 
background, even on single user systems.

I mean it's something we can consider, we even used to have code to do it 
(called factory mode), but I don't it's a
absolute given that we should do it at this point.

 systemd-consoled is an example how graphics applications work with
 TakeControl()/TakeDevice(). It expects to be run inside a session, so
 the session has to be setup by the parent beforehand.
 systemd-welcomed is an example how login-screens can work. It spawns a
 separate process for the greeter and one for each session. Note that
 the greeter runs as user nobody *inside* of a systemd-logind
 session. So it's the same as any other session. Not like gdm which
 treats the greeter special.
gdm runs the greeter inside a logind session as a gdm user, too.  It does
this by invoking a minimal pam stack to open the session which runs pam_systemd.

The X server doesn't run inside the session at the moment though.

--Ray
___
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: Using systemd-logind Session.TakeControl() from Xorg, input needed

[Ray Strode]

Hi,

- Original Message -
 No it ain't. If you use DRM, you can run a smooth handover without
 tearing, flicker or other artifacts. It's not obvious and not
 documented at all (sadly), but it's possible. You simply avoid doing a
 full/deep modeset on session activation but instead take-over where it
 was left. Schedule your framebuffer for the next vblank and you will
 see no artifacts. Obviously, if you need to change (pixel-)modes or
 clock parameters, a full modeset is required. But that's always the
 case, whether you use a system-compositor or not.
The problem is the drivers are notoriously bad at keeping -background none 
working.
The intel driver only works if using SNA, and the modesetting driver doesn't 
work
at all, so we're going to get black a lot of the time when the X server first 
starts
up.

That's the situation with plymouth - X right now anyway.

  2) It will mean there's always an X server with a login screen running in
  the background, even on single user systems.
 
 You can shut down the greeter-session after login and re-create it
 when required again.
You can't shut down an X server unless it's in the foreground.

  I mean it's something we can consider, we even used to have code to do it
  (called factory mode), but I don't it's a
  absolute given that we should do it at this point.
 
 I'm lost, sorry. So you are saying we should try it or not?
You asked why not, and I listed some reasons why it isn't an obvious yes or no
on which way to go.  I'm on the fence.  It's worth at least experimenting with.
We're essentially going to have to do that anyway for wayland sessions (since 
the greeter
itself doesn't run over wayland right now).
 
 I skimmed through the gdm code but didn't find the pam-transaction for
 the gdm user, only the one for the new sessions. Thanks for clarifying
 that! It's the right thing to do, imho.

The GDM code is a bit on the complicated side...

Greeter is started here:

https://git.gnome.org/browse/gdm/tree/daemon/gdm-simple-slave.c#n1277

which ultimately calls gdm_launch_environment_start, which creates a GdmSession 
object here:

https://git.gnome.org/browse/gdm/tree/daemon/gdm-launch-environment.c#n467

The GdmSession object is what fires off a gdm-session-worker process to run the 
PAM conversation and launch a session.

If you have a login screen up, you can see the worker process by doing

ps -ef |grep gdm-session-worker

and you'll see something like gdm-session-worker [pam/gdm-launch-environment] 
in the ps listing

--Ray
___
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: [PATCH] config: provide example configuration for multi-seat setups

[Ray Strode]

Hi,

(sorry for the slow response on this)

On Fri, Jul 12, 2013 at 1:32 PM, Lennart Poettering
lenn...@poettering.netwrote:

 Ray, this would mean gdm would no longer have to call our multi-seat-x
 tool for seats != seat0 but rather use the normal X binary but pass
 -config non-seat0.conf.multi-seat to it. Ray, does this look good to
 you?

Sure, that's fine, but what about the other stuff that ends up in the
config file written by the systemd script?

Option \AutoAddDevices\ \True\\n
Option \AllowEmptyInput\ \True\\n

and what about -sharedvts ?

--Ray
___
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: [PATCH 2/2] Xinit: close stdin to avoid leak of file descriptior to the Xorg session.

[Ray Strode]

Hi,

Apparently I wrote this patch like 5 years and promptly forgot about.
Matej tracked me down and asked me to chime in.  I don't really
remember the patch, but it sort of makes sense to me so I'll try to
answer any questions about it.

 What's the point of setsid(2)?  It certainly doesn't fall under the title of 
 the patch.

Looking through version control, the original message in the patch is:

  - start client in its own session with no controlling tty (bug 214649)

This is also a bit terse, sadly, but a little clearer.  The idea is,
when you start an X server, the tty the X server started on is pretty
irrelevant as far as the clients are concerned  Even the X server
itself is going to be running on its own VT switched away from the tty
it was started on.  Given the tty doesn't matter, the path tries to
remove remnants of that tty from the client's view.

The code previously did setpgid(0, 0) which just means put the client
in its own process group in the same session as the shell it was
started from.  But the shell xinit is started from is irrelevant to
the clients running on the X server.  Process group is roughly
synonymous with shell job  Taken in that light, we wouldn't want
xinit to be job %1 and xterm or gnome-session or whatever to be job %2
at the sh prompt the user typed startx from.  That makes no sense and
has weird unix-y ramifications (if the client or one of the clients
descendants tries to read from stdin it's going to get stopped with
SIGTTIN since it might not be in the foreground process group).

By using setsid() we make sure the X session initiated with that first
X client is started in its own terminal session independent of the
shell it was started from.  We still leave stdout/stderr hooked up to
the tty since multiple sessions can output to a tty without much
trouble, and then client output doesn't get sent into the aether.
Redirecting to ~/.xsession-errors would be fine too.


 Also, you don't need:
 +         close (STDIN_FILENO);
true, there are times when you should explicitly close before dup2()
(if you need to do proper error checking on close()), but this isn't
one of them

 That is done for you by dup2(2).  Also doing so could result in a problem  
 if this code were used in a multithreaded application.  If another thread
 called open(2) between the close(2) and the dup2(2), it would get
 STDIN_FILENO as its fd and you would then close it with your dup2(2).
xinit isn't multithreaded, though, and even if it were, this is less
than a dozen lines down from a fork(). so the function itself is
necessarily running in a single thread.

--Ray
___
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: [PATCH 2/2] Xinit: close stdin to avoid leak of file descriptior to the Xorg session.

[Ray Strode]

Hi,

On Tue, Jul 26, 2011 at 2:57 PM, Jeremy Huddleston jerem...@apple.com wrote:
 yick... ok, well I'd suggest making that its own patch rather than squashed  
 into the stdin one... I dunno how I feel about it and will let others chime 
 in...
It's two pieces of the same thing.  There's not much point in setting
stdin to /dev/null but leaving the controlling tty the same.  The
point of all the steps (as I see it anyway) is a common end goal of
detaching the new X session that's getting launched from the existing
session running on the tty. Another way you could think of it, is it's
basically doing the same fork()/close/dup2/setsid dance a libc
daemon() call would do but without the final exit-in-parent.

--Ray
___
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: [PATCH 2/2] Xinit: close stdin to avoid leak of file descriptior to the Xorg session.

[Ray Strode]

Hi,

On Tue, Jul 26, 2011 at 4:02 PM, Jeremy Huddleston jerem...@apple.com wrote:
 IMO, there is a point to closing stdin aside from the setsid(2).
My point is, it only solves the problem part way.

As an example, say a program wants to ask the user for a password.
The program supports asking the user at the console if run from a tty,
and supports asking the user from an X dialog otherwise.  The way that
program would ask the user for a password at the console is by opening
/dev/tty (since password programs don't read input from stdin).  That
program could first try to open /dev/tty, and if it fails assume an X
fall back.  If you haven't insulated the client from the tty startx
was run on, then this program may end up trying to ask for a password
on some switched away VT! and would probably get suspended instantly
with SIGTTIN.  You could argue the client should try X first and fall
back to console.  Or you could argue the client should do isatty() on
stdin before trying to open /dev/tty.  But both are debatable and this
is just one example, anyway.

The example serves to show that redirecting STDIN to /dev/null
partially solves the same problem setsid partially solves.That problem
is detaching X clients from the tty startx was run on.

Or is there another problem being solved, that you have in mind?

--Ray
___
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: [PATCH 2/2] Xinit: close stdin to avoid leak of file descriptior to the Xorg session.

[Ray Strode]

Hi,

On Tue, Jul 26, 2011 at 4:42 PM, Lennart Poettering
lenn...@poettering.net wrote:
 What I am trying to say here basically: if you are planning to invoke
 setsid() or detach from the controlling tty otherwise in the normal X
 servers, then you'll break existing code. And I'd like to ask you not to
 do that...
...
 But then again, I didn't really follow the discussion and maybe you are
 discussing something compltely different.
Yea, we're talking about something different.  xinit does two things
1) start the server 2) start an initial client

You're worried about 1, we're talking about 2.

--Ray
___
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: [PATCH 2/2] Xinit: close stdin to avoid leak of file descriptior to the Xorg session.

[Ray Strode]

Hi,
 Well, the problem you are reporting is 2 parted, hence you should have 2  
 commits.  The stdin closing is an issue in its own right and should be a
 separate patch.
Okay, i'll look into splitting up the patch and follow up to your
other points later.

--Ray
___
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