[PATCH xserver] Fix a segfault that occurs if xorg.conf.d is absent:

2016-11-15 Thread Ben Crocker 
In InitOutput, if xf86HandleConfigFile returns CONFIG_NOFILE
(which it does if no config file or directory is present), the
autoconfig flag is set, causing xf86AutoConfig to be called
later on.

xf86AutoConfig calls xf86OutputClassDriverList via the
call tree:

xf86AutoConfig =>
  listPossibleVideoDrivers =>
xf86PlatformMatchDriver =>
  xf86OutputClassDriverList

and xf86OutputClassDriverList attempts to traverse a linked list
that is a member of the XF86ConfigRec struct pointed to by the
global xf86configptr, which is NULL at this point because the
XF86ConfigRec struct is only allocated (by xf86readConfigFile)
AFTER the config file and directory have been successfully
opened; the CONFIG_NOFILE return from xf86HandleConfigFile
occurs BEFORE the call to xf86readConfigFile which allocates
the XF86ConfigRec struct.

Rx: In read.c (for symmetry with xf86freeConfig, which already
appears in this file), add a new function xf86allocateConfig
which tests the value of xf86configptr and, if it's NULL,
allocates the XF86ConfigRec struct and deposits the pointer
in xf86configptr.  In xf86Parser.h, add a prototype for the
new xf86allocateConfig function.

Back in read.c, #include "xf86Config.h".  In xf86readConfigFile,
change the open-code call to calloc to a call to the new
xf86allocateConfig function.

In xf86AutoConfig.c, add a call to the new xf86allocateConfig function
to the beginning of xf86AutoConfig to make sure the XF86ConfigRec struct
is allocated.

Signed-off-by: Ben Crocker 
---
 hw/xfree86/common/xf86AutoConfig.c |  9 +
 hw/xfree86/parser/read.c   | 16 +++-
 hw/xfree86/parser/xf86Parser.h |  1 +
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/hw/xfree86/common/xf86AutoConfig.c 
b/hw/xfree86/common/xf86AutoConfig.c
index 9402651..c3e17be 100644
--- a/hw/xfree86/common/xf86AutoConfig.c
+++ b/hw/xfree86/common/xf86AutoConfig.c
@@ -149,6 +149,15 @@ xf86AutoConfig(void)
 char buf[1024];
 ConfigStatus ret;
 
+/* Make sure config rec is there */
+if (xf86allocateConfig() != NULL) {
+ret = CONFIG_OK;/* OK so far */
+}
+else {
+xf86Msg(X_ERROR, "Couldn't allocate Config record.\n");
+return FALSE;
+}
+
 listPossibleVideoDrivers(deviceList, 20);
 
 for (p = deviceList; *p; p++) {
diff --git a/hw/xfree86/parser/read.c b/hw/xfree86/parser/read.c
index ec038ae..d7e7312 100644
--- a/hw/xfree86/parser/read.c
+++ b/hw/xfree86/parser/read.c
@@ -56,6 +56,7 @@
 #include 
 #endif
 
+#include "xf86Config.h"
 #include "xf86Parser.h"
 #include "xf86tokens.h"
 #include "Configint.h"
@@ -91,7 +92,7 @@ xf86readConfigFile(void)
 int token;
 XF86ConfigPtr ptr = NULL;
 
-if ((ptr = calloc(1, sizeof(XF86ConfigRec))) == NULL) {
+if ((ptr = xf86allocateConfig()) == NULL) {
 return NULL;
 }
 
@@ -270,6 +271,19 @@ xf86itemNotSublist(GenericListPtr list_1, GenericListPtr 
list_2)
 return (!(last_1 == last_2));
 }
 
+/*
+ * Conditionally allocate config struct, but only allocate it
+ * if it's not already there.  In either event, return the pointer
+ * to the global config struct.
+ */
+XF86ConfigPtr xf86allocateConfig(void)
+{
+if (!xf86configptr) {
+xf86configptr = calloc(1, sizeof(XF86ConfigRec));
+}
+return xf86configptr;
+}
+
 void
 xf86freeConfig(XF86ConfigPtr p)
 {
diff --git a/hw/xfree86/parser/xf86Parser.h b/hw/xfree86/parser/xf86Parser.h
index ff35846..9c4b403 100644
--- a/hw/xfree86/parser/xf86Parser.h
+++ b/hw/xfree86/parser/xf86Parser.h
@@ -449,6 +449,7 @@ extern char *xf86openConfigDirFiles(const char *path, const 
char *cmdline,
 extern void xf86setBuiltinConfig(const char *config[]);
 extern XF86ConfigPtr xf86readConfigFile(void);
 extern void xf86closeConfigFile(void);
+extern XF86ConfigPtr xf86allocateConfig(void);
 extern void xf86freeConfig(XF86ConfigPtr p);
 extern int xf86writeConfigFile(const char *, XF86ConfigPtr);
 extern _X_EXPORT XF86ConfDevicePtr xf86findDevice(const char *ident,
-- 
2.7.4

___
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 util-modular 6/6] release.sh: run ./configure within the release.sh

2016-11-15 Thread Emil Velikov 
On 14 November 2016 at 20:54, Peter Hutterer  wrote:
> On Mon, Nov 14, 2016 at 03:39:18PM +, Emil Velikov wrote:
>> On 13 November 2016 at 22:24, Peter Hutterer  
>> wrote:
>> > On Fri, Nov 11, 2016 at 03:45:29PM +, Emil Velikov wrote:
>> >> From: Emil Velikov 
> [...]
>> >> +return 1
>> >>  fi
>> >> -build_dir=`dirname $status_file`
>> >> +
>> >>  cd $build_dir
>> >>  if [ $? -ne 0 ]; then
>> >>   echo "Error: failed to cd to $MODULE_RPATH/$build_dir."
>> >> @@ -377,6 +373,15 @@ process_module() {
>> >>   return 1
>> >>  fi
>> >>
>> >> +# Using ../ here feels a bit nasty, yet $top_src is an absolute 
>> >> path. Thus
>> >> +# it will get propagated in the generated sources, which we do not 
>> >> want.
>> >> +../configure >/dev/null
>> >> +if [ $? -ne 0 ]; then
>> >> +echo "Error: failed to configure module."
>> >> +cd $top_src
>> >
>> > I'd really like to see more pushd/popd, but that's unrelated to this patch.
>> > Looks good, but I think you should look at the effort to do a full local
>> > clone, it may only be a few lines and it should remove quite a bit of the
>> > error checking.
>> >
>> Yes I'm thinking/working towards having things in a cleaner state. At
>> the same time this patch would cause a noticeable change in workflow,
>> so I've intentionally opted out of making things too intrusive.
>>
>> Any objections against git worktree and/or moving towards it as a
>> follow-up change ?
>> Or you'd thinking this patch should be "it's all or nothing" kind of change ?
>
> simply said, I don't know enough about worktrees to have an opinion here,
> sorry. for the mere tarball generation and distcheck run you can use a
> shallow clone though, can't you? Just something to keep in mind, if the
> worktree works as a clean checkout without detritus, I'll be happy with tha
> too.
>
In a line: worktree like a fresh clone, with the only difference being
that .git is a file pointing to the actual git repo.
Here is an example and more info [1].

Can we have all that [shallow/full clone, worktree, other] as
follow-up, please ? Or you really don't like the git clean suggestion
?
The current patch/workflow has been tested for over a dozen times and
I feel a bit reluctant in pushing for something that's barely tested
:-\

Thanks
Emil

[1] https://git-scm.com/docs/git-worktree#_examples
___
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 util-modular 2/6] release.sh: add a couple more info messages

2016-11-15 Thread Emil Velikov 
On 14 November 2016 at 20:55, Peter Hutterer  wrote:
> On Mon, Nov 14, 2016 at 03:42:04PM +, Emil Velikov wrote:
>> On 13 November 2016 at 22:11, Peter Hutterer  
>> wrote:
>> > On Fri, Nov 11, 2016 at 03:45:25PM +, Emil Velikov wrote:
>> >> From: Emil Velikov 
>> >>
>> >> This way one can relate the exact stage they are and the possible
>> >> password prompt they'll see (if the ssh/gpg agent has timed out).
>> >>
>> >> Signed-off-by: Emil Velikov 
>> >> ---
>> >>  release.sh | 2 ++
>> >>  1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/release.sh b/release.sh
>> >> index 620b5a4..95050ca 100755
>> >> --- a/release.sh
>> >> +++ b/release.sh
>> >> @@ -580,6 +580,7 @@ process_module() {
>> >>  # srv_path="~/public_html$srv_path"
>> >>
>> >>  # Check that the server path actually does exist
>> >> +echo "Info: check in path exists on web server:"
>> >
>> > This should be "checking" right?
>> > For patches 1-5: Reviewed-by: Peter Hutterer 
>> >
>> Tyvm for addressing these so quick Peter !
>>
>> Should I wait for 6/6 to reach conclusion or you'd like a resent with
>> the above sorted ?
>
> nah, I reckon you can push these directly. Or tell me to do it if you don't
> have push access.
>
Don't have access I'm afraid. I'm "only" in the freedesktop dri mesa
piglit groups.

Thanks
Emil
___
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

[PATCH xserver] xwayland: Fix use after free of cursors

2016-11-15 Thread Olivier Fourdan 
Sometimes, Xwayland will try to use a cursor that has just been freed,
leading to a crash when trying to access that cursor data either in
miPointerUpdateSprite() or elsewhere.

This issue is very random and hard to reproduce.

Typical backtraces include:

  miPointerUpdateSprite () at mipointer.c
  mieqProcessInputEvents () at mieq.c
  ProcessInputEvents () at xwayland-input.c
  Dispatch () at dispatch.c
  dix_main () at main.c

or

  miPointerUpdateSprite () at mipointer.c
  mieqProcessInputEvents () at mieq.c
  keyboard_handle_modifiers () at xwayland-input.c
  wl_closure_invoke () at src/connection.c
  dispatch_event () at src/wayland-client.c
  dispatch_queue () at src/wayland-client.c
  wl_display_dispatch_queue_pending () at src/wayland-client.c
  wl_display_dispatch_pending () at src/wayland-client.c
  xwl_read_events () at xwayland.c
  WaitForSomething () at WaitFor.c
  Dispatch () at dispatch.c
  dix_main () at main.c

or

  AnimCurTimerNotify () at animcur.c
  DoTimer () at WaitFor.c
  DoTimers () at WaitFor.c
  check_timers () at WaitFor.c
  WaitForSomething () at WaitFor.c
  Dispatch () at dispatch.c
  dix_main () at main.c

CheckMotion() would update the pointer's cursor only when the sprite
windows differ before and after calling XYToWindow(), but Xwayland
implements its own xwl_xy_to_window() which would fake a crossing to
the root window if the pointer has left the Wayland surface but is still
within the Xwindow, which confuses CheckMotion().

Typically, after the cursors have been freed from CloseDownClient(), if
the pointer's sprite window is already the root window, and Xwayland's
xwl_xy_to_window() fakes a transition to the root window as well, the
previous and new sprite windows are already identical and CheckMotion()
will not call PostNewCursor() and thus not invoke
miPointerDisplayCursor() that would have updated the pointer's cursor.

Any further attempt to update the pointer using that cursor will lead to
a crash.

To avoid this issue, modify Xwayland's own xwl_xy_to_window() to avoid
returning the root window if the sprite window is already the root
window, so that the logic in CheckMotion() is preserved.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1385258
Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland-input.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
index 0526122..88c12f5 100644
--- a/hw/xwayland/xwayland-input.c
+++ b/hw/xwayland/xwayland-input.c
@@ -1256,7 +1256,8 @@ sprite_check_lost_focus(SpritePtr sprite, WindowPtr 
window)
  */
 if (master->lastSlave == xwl_seat->pointer &&
 xwl_seat->focus_window == NULL &&
-xwl_seat->last_xwindow == window)
+xwl_seat->last_xwindow == window &&
+sprite->win != sprite->spriteTrace[0])
 return TRUE;
 
 xwl_seat->last_xwindow = window;
-- 
2.9.3

___
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 xf86-input-libinput] If the parent libinput_device is unavailable, create a new one

2016-11-15 Thread Hans de Goede 

Hi,

On 15-11-16 05:37, Peter Hutterer wrote:

The parent device ref's the libinput device during pre_init and unref's it
during DEVICE_INIT, so the copy is lost. During DEVICE_ON, the libinput device
is re-added and ref'd, this one stays around now. But the takeaway is: unless
the device is enabled, no libinput device reference is available.

If a device is a mixed pointer + keyboard device, a subdevice is created
during a WorkProc. The subdevice relied on the parent's libinput_device being
available and didn't even check for it. This WorkProc usually runs after
the parent's DEVICE_ON, so in most cases all is well.

But when running without logind and the server is vt-switched away, the parent
device only runs PreInit and DEVICE_INIT but never DEVICE_ON, causing the
subdevice to burn, crash, and generally fail horribly when it dereferences the
parent's libinput device.

Fix this because we have global warming already and don't need to burn more
things and also because it's considered bad user experience to have the
server crash. The simple fix is to check the parent device first and if it is
unavailable, create a new one because it will end up disabled as well anyway,
so the ref goes away as well. The use-case where the parent somehow gets
disabled but the subdevice doesn't is a bit too niche to worry about.

This doesn't happen with logind because in that case we don't get a usable fd
while VT-switched away, so we can't even run PreInit and never get this far
(see the paused fd handling in the xfree86 code for that). It can be
reproduced by setting AutoEnableDevices off, but why would you do that,
seriously.

https://bugs.freedesktop.org/show_bug.cgi?id=97117

Signed-off-by: Peter Hutterer 


Hmm, so what happens if the parent device later does get DEVICE_ON, after
the subdevice has created its own libinputdevice ?

Regards,

Hans




---
 src/xf86libinput.c | 42 +++---
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/xf86libinput.c b/src/xf86libinput.c
index 6792d1c..747e84b 100644
--- a/src/xf86libinput.c
+++ b/src/xf86libinput.c
@@ -2850,7 +2850,7 @@ xf86libinput_pre_init(InputDriverPtr drv,
struct xf86libinput *driver_data = NULL;
struct xf86libinput_device *shared_device = NULL;
struct libinput *libinput = NULL;
-   struct libinput_device *device;
+   struct libinput_device *device = NULL;
char *path = NULL;
bool is_subdevice;

@@ -2885,7 +2885,28 @@ xf86libinput_pre_init(InputDriverPtr drv,
}

is_subdevice = xf86libinput_is_subdevice(pInfo);
-   if (!is_subdevice) {
+   if (is_subdevice) {
+   InputInfoPtr parent;
+   struct xf86libinput *parent_driver_data;
+
+   parent = xf86libinput_get_parent(pInfo);
+   if (!parent) {
+   xf86IDrvMsg(pInfo, X_ERROR, "Failed to find parent 
device\n");
+   goto fail;
+   }
+
+   parent_driver_data = parent->private;
+   if (!parent_driver_data) /* parent already removed again */
+   goto fail;
+
+   xf86IDrvMsg(pInfo, X_INFO, "is a virtual subdevice\n");
+   shared_device = 
xf86libinput_shared_ref(parent_driver_data->shared_device);
+   device = shared_device->device;
+   if (!device)
+   xf86IDrvMsg(pInfo, X_ERROR, "Parent device not 
available\n");
+   }
+
+   if (!device) {
device = libinput_path_add_device(libinput, path);
if (!device) {
xf86IDrvMsg(pInfo, X_ERROR, "Failed to create a device for 
%s\n", path);
@@ -2903,23 +2924,6 @@ xf86libinput_pre_init(InputDriverPtr drv,
libinput_device_unref(device);
goto fail;
}
-   } else {
-   InputInfoPtr parent;
-   struct xf86libinput *parent_driver_data;
-
-   parent = xf86libinput_get_parent(pInfo);
-   if (!parent) {
-   xf86IDrvMsg(pInfo, X_ERROR, "Failed to find parent 
device\n");
-   goto fail;
-   }
-
-   parent_driver_data = parent->private;
-   if (!parent_driver_data) /* parent already removed again */
-   goto fail;
-
-   xf86IDrvMsg(pInfo, X_INFO, "is a virtual subdevice\n");
-   shared_device = 
xf86libinput_shared_ref(parent_driver_data->shared_device);
-   device = shared_device->device;
}

pInfo->private = driver_data;


___
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 v4 xserver] xkb: fix releasing overlay while keydown

2016-11-15 Thread Mariusz Mazur 
A week has passed and everything's working fine (and I'm using the
overlay keycombos very very heavily). Seems the patch does not have
any unforeseen side effects.

2016-11-07 22:25 GMT+01:00 Mariusz Mazur :
> Applied to my work env. If something starts acting funny I'll let you know.
>
> 2016-11-06 23:42 GMT+01:00 Mihail Konev :
>> Testcase:
>>
>> In ~/.xbindkeysrc:
>>
>>   "xterm &"
>>XF86LaunchA
>>
>> In ~/ov.xkb:
>>
>>   xkb_keymap {
>>   xkb_keycodes { include "evdev" };
>>   xkb_types{ include "complete" };
>>
>>   xkb_compat   { include "complete"
>>interpret Overlay1_Enable+AnyOfOrNone(all) {
>>   action= SetControls(controls=Overlay1);
>>};
>>   };
>>
>>   xkb_symbols  { include "pc+inet(evdev)+us"
>>   key  { [ Overlay1_Enable ] };
>>   key  { overlay1 =  }; // Insert+1 => 2
>>   key  { overlay1 =  }; // Insert+~ => 
>> XF86LaunchA
>>   };
>>
>>   xkb_geometry { include "pc(pc104)" };
>>   };
>>
>> Apply this layout: 'xkbcomp ~/ov.xkb $DISPLAY'.
>> Run "xbindkeys -n -v"
>> In the exact order:
>> - press Insert
>> - press Tilde
>> - release Insert
>> - wait
>> - release Tilde
>> Keyboard input in the new terminal window(s) would be locked
>> until another Insert+Tilde .
>>
>> Reported-by: Mariusz Mazur 
>> Signed-off-by: Mihail Konev 
>> ---
>> v3 was still incorrect and did not done what it was supposed to.
>> This version is specifically tested to properly enable and disable
>> overlay, i.e. allow "`"-s to be sent both before and after Insert being
>> down.
>> Debugging version attached.
>>
>> Without (keywas_overlaid - 1) trickery it does not address the issue
>> (i.e. input stays locked until Insert+Tilde)
>> (but does not happen without open-new-window being triggered by xbindkeys,
>> i.e. when the latter is not running).
>> Maybe overlay_perkey_state description comment should better reflect this.
>>
>> Also commit description missed Reported-by.
>>
>> The "where-overlay1,2-is-in-xkb" is resolved in this patch.
>>
>> As for "applicability of overlays", they are per-keycode, and 
>> layout-independent.
>> This differs from RedirectKey that are per-keysym, and, therefore,
>> also per-shift-level and per-layout (per-group).
>>
>> There should be no need to use overlays instead of RedirectKey,
>> especially given that overlay is a "behavior", which
>> could be only one per keycode.
>>
>>  xkb/xkbPrKeyEv.c | 36 +++-
>>  1 file changed, 31 insertions(+), 5 deletions(-)
>>
>> diff --git a/xkb/xkbPrKeyEv.c b/xkb/xkbPrKeyEv.c
>> index f7a6b4b14306..35bb1e9f405a 100644
>> --- a/xkb/xkbPrKeyEv.c
>> +++ b/xkb/xkbPrKeyEv.c
>> @@ -43,6 +43,14 @@ THE USE OR PERFORMANCE OF THIS SOFTWARE.
>>
>>  /******/
>>
>> +/* Keeps track of overlay in effect for a given key,
>> + * so that if an overlay is released while key is down,
>> + * the key retains overlaid until its release.
>> + * Cannot be a bitmask, as needs at least three values
>> + * (as overlaid keys need to generate two releases).
>> + * */
>> +static unsigned char overlay_perkey_state[256];
>> +
>>  void
>>  XkbProcessKeyboardEvent(DeviceEvent *event, DeviceIntPtr keybd)
>>  {
>> @@ -121,20 +129,38 @@ XkbProcessKeyboardEvent(DeviceEvent *event, 
>> DeviceIntPtr keybd)
>>  case XkbKB_Overlay2:
>>  {
>>  unsigned which;
>> +unsigned overlay_active_now;
>> +unsigned is_keyrelease = (event->type == ET_KeyRelease) ? 1 : 0;
>> +unsigned key_was_overlaid = 0;
>>
>>  if (behavior.type == XkbKB_Overlay1)
>>  which = XkbOverlay1Mask;
>>  else
>>  which = XkbOverlay2Mask;
>> -if ((xkbi->desc->ctrls->enabled_ctrls & which) == 0)
>> +overlay_active_now = (xkbi->desc->ctrls->enabled_ctrls & which) 
>> ? 1 : 0;
>> +
>> +if ((unsigned char)key == key) {
>> +key_was_overlaid = overlay_perkey_state[key];
>> +if (overlay_active_now && !is_keyrelease)
>> +overlay_perkey_state[key] = 2;
>> +else if (is_keyrelease && (overlay_active_now || 
>> key_was_overlaid))
>> +overlay_perkey_state[key] = key_was_overlaid - 1;
>> +else if (key_was_overlaid && !overlay_active_now && 
>> !is_keyrelease) {
>> +/* ignore key presses after overlay is released,
>> + * as their release would have been overridden in prev 
>> branch,
>> + * and key would need another key-and-release to 
>> recover from overlay
>> + * */
>> +return;
>> +} else {
>> +break;
>> +