date:20171027

Re: [PATCH xserver] present: Only send PresentCompleteNotify events to the presenting client

2017-10-27 Thread Keith Packard

Michel Dänzer  writes:

> Turns out it's way worse than that. The PresentPixmap and
> PresentNotifyMSC requests don't take a PRESENTEVENTID parameter.
> Instead, present_send_idle/complete_notify send every event to every
> client which called PresentSelectInput for the window with the event
> kind enabled in the event mask, and always set the event ID specified in
> PresentSelectInput in the event sent to each client. This defeats the
> purpose of the event ID and results in all events getting sent to and
> processed by all listening clients.

Well, it certainly makes the event ID do something different at least...

I think we should adopt your suggestion and make these events work like
GraphicsExpose and be sent only to the client who executed the
associated request.

-- 
-keith


signature.asc
Description: PGP signature
___
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] present: Only send PresentCompleteNotify events to the presenting client

2017-10-27 Thread Michel Dänzer

On 20/10/17 09:58 PM, Keith Packard wrote:
> Michel Dänzer  writes:
> 
>> From: Michel Dänzer 
>>
>> We were sending the events to all clients listening for them on the
>> window. But clients can get confused by events from another client, and
>> I can't imagine any case where reciving events from other clients would
>> be required.
> 
> While I agree that it's unlikely to be useful to send the event to all
> listening clients, there is a 'PRESENTEVENTID' in the CompleteNotify
> which clients "should" be using to check to see if the event being
> delivered is the one associated with their action.
> 
> Did we mess up in some client library and not do this?

Turns out it's way worse than that. The PresentPixmap and
PresentNotifyMSC requests don't take a PRESENTEVENTID parameter.
Instead, present_send_idle/complete_notify send every event to every
client which called PresentSelectInput for the window with the event
kind enabled in the event mask, and always set the event ID specified in
PresentSelectInput in the event sent to each client. This defeats the
purpose of the event ID and results in all events getting sent to and
processed by all listening clients.

-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

signature.asc
Description: OpenPGP digital signature
___
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] xf86-video-modesetting: Fix ms_queue_vblank(flags = MS_QUEUE_RELATIVE)

2017-10-27 Thread Adam Jackson

On Thu, 2017-10-26 at 20:51 -0700, Keith Packard wrote:
> Alex Goins  writes:
> 
> > Adding Keith, as this is a regression that completely breaks PRIME Sync 
> > configs
> > at top of tree.
> 
> Reviewed-by: Keith Packard 

remote: I: patch #183918 updated using rev 
266d9868ca1cf77b7d315d607b515f081a9f45c3.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   68d95e759f..266d9868ca  master -> master

- ajax
___
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 1/4] modesetting: Fix potential buffer overflow

2017-10-27 Thread Daniel Martin

If one misconfigures a ZaphodHeads value (more than 20 characters
without a delimiter), we get an overflow of our buffer.
Use xstrtokenize() instead of writing/fixing our own tokenizer.

Signed-off-by: Daniel Martin 
---
 hw/xfree86/drivers/modesetting/drmmode_display.c | 38 
 1 file changed, 13 insertions(+), 25 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 5bfae0b03..e14833dee 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -57,34 +57,22 @@ static PixmapPtr drmmode_create_pixmap_header(ScreenPtr 
pScreen, int width, int
 static Bool
 drmmode_zaphod_string_matches(ScrnInfoPtr scrn, const char *s, char 
*output_name)
 {
-int i = 0;
-char s1[20];
+char **token = xstrtokenize(s, ", \t\n\r");
+Bool ret = FALSE;
 
-do {
-switch(*s) {
-case ',':
-s1[i] = '\0';
-i = 0;
-if (strcmp(s1, output_name) == 0)
-return TRUE;
-break;
-case ' ':
-case '\t':
-case '\n':
-case '\r':
-break;
-default:
-s1[i] = *s;
-i++;
-break;
-}
-} while(*s++);
+if (!token)
+return FALSE;
 
-s1[i] = '\0';
-if (strcmp(s1, output_name) == 0)
-return TRUE;
+for (int i = 0; token[i]; i++) {
+if (strcmp(token[i], output_name) == 0)
+ret = TRUE;
 
-return FALSE;
+free(token[i]);
+}
+
+free(token);
+
+return ret;
 }
 
 int
-- 
2.13.6

___
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 4/4] Use ARRAY_SIZE all over the tree

2017-10-27 Thread Daniel Martin

Roundhouse kick replacing the various (sizeof(foo)/sizeof(foo[0])) with
the ARRAY_SIZE macro from dix.h when possible.
A semantic patch for coccinelle has been used first. Additionally, few
macros have been inlined as they had only one or two users.

Signed-off-by: Daniel Martin 
---
Just wanted to replace MS_ARRAY_SIZE in modesetting, but couldn't resist
to touch the whole tree and at least found a bug in a test. So, the
effort is justified - for me.
Couldn't (compile) test xquartz and xwin. Though, the changes in
general should be okay, except if the include to dix.h is missing.

 Xext/geext.c |  3 +--
 Xext/saver.c |  6 ++
 Xi/xiproperty.c  |  6 ++
 composite/compinit.c |  5 +
 damageext/damageext.c|  4 +---
 dix/eventconvert.c   |  4 +---
 dix/gc.c |  2 +-
 dix/privates.c   |  4 +---
 glamor/glamor_utils.h|  4 
 hw/kdrive/src/kdrive.c   |  4 +---
 hw/xfree86/common/modeline2c.awk |  2 +-
 hw/xfree86/common/xf86Configure.c|  2 +-
 hw/xfree86/common/xf86Mode.c |  4 ++--
 hw/xfree86/dri2/dri2.c   |  2 +-
 hw/xfree86/drivers/modesetting/drmmode_display.c |  2 +-
 hw/xfree86/drivers/modesetting/drmmode_display.h |  2 --
 hw/xfree86/loader/loadmod.c  |  3 ++-
 hw/xfree86/modes/xf86EdidModes.c |  6 ++
 hw/xfree86/os-support/bsd/bsd_apm.c  |  4 +---
 hw/xfree86/os-support/bsd/bsd_init.c |  2 +-
 hw/xfree86/os-support/bsd/bsd_kqueue_apm.c   |  4 +---
 hw/xfree86/os-support/linux/lnx_apm.c|  6 ++
 hw/xfree86/os-support/shared/posix_tty.c |  2 +-
 hw/xfree86/os-support/solaris/sun_apm.c  |  4 +---
 hw/xquartz/darwin.c  |  5 ++---
 hw/xquartz/keysym2ucs.c  |  6 +++---
 hw/xquartz/quartzKeyboard.c  | 10 --
 hw/xquartz/xpr/x-hash.c  |  4 +---
 hw/xwin/InitOutput.c |  6 ++
 hw/xwin/glx/indirect.c   |  6 ++
 hw/xwin/winclipboard/wndproc.c   |  2 +-
 hw/xwin/winclipboard/xevents.c   |  3 +--
 hw/xwin/winmultiwindowwm.c   |  2 +-
 os/auth.c|  3 +--
 os/oscolor.c |  5 ++---
 render/glyph.c   |  2 +-
 test/input.c |  2 +-
 test/signal-logging.c|  8 
 test/tests-common.h  |  2 ++
 xfixes/cursor.c  |  4 +---
 xfixes/xfixes.c  |  4 +---
 xkb/xkbDflts.h   |  4 ++--
 42 files changed, 60 insertions(+), 105 deletions(-)

diff --git a/Xext/geext.c b/Xext/geext.c
index 08dff280b..5009c081a 100644
--- a/Xext/geext.c
+++ b/Xext/geext.c
@@ -47,7 +47,6 @@ static const int version_requests[] = {
 /* Forward declarations */
 static void SGEGenericEvent(xEvent *from, xEvent *to);
 
-#define NUM_VERSION_REQUESTS   (sizeof (version_requests) / sizeof 
(version_requests[0]))
 #define EXT_MASK(ext) ((ext) & 0x7F)
 
 //
@@ -127,7 +126,7 @@ ProcGEDispatch(ClientPtr client)
 
 REQUEST(xGEReq);
 
-if (pGEClient->major_version >= NUM_VERSION_REQUESTS)
+if (pGEClient->major_version >= ARRAY_SIZE(version_requests))
 return BadRequest;
 if (stuff->ReqType > version_requests[pGEClient->major_version])
 return BadRequest;
diff --git a/Xext/saver.c b/Xext/saver.c
index f6090d8da..4d9e6b974 100644
--- a/Xext/saver.c
+++ b/Xext/saver.c
@@ -1268,14 +1268,12 @@ ProcScreenSaverQueryVersion,
 ProcScreenSaverSetAttributes,
 ProcScreenSaverUnsetAttributes, ProcScreenSaverSuspend,};
 
-#define NUM_REQUESTS   ((sizeof NormalVector) / (sizeof NormalVector[0]))
-
 static int
 ProcScreenSaverDispatch(ClientPtr client)
 {
 REQUEST(xReq);
 
-if (stuff->data < NUM_REQUESTS)
+if (stuff->data < ARRAY_SIZE(NormalVector))
 return (*NormalVector[stuff->data]) (client);
 return BadRequest;
 }
@@ -1360,7 +1358,7 @@ SProcScreenSaverDispatch(ClientPtr client)
 {
 REQUEST(xReq);
 
-if (stuff->data < NUM_REQUESTS)
+if (stuff->data < ARRAY_SIZE(NormalVector))
 return (*SwappedVector[stuff->data]) (client);
 return BadRequest;
 }
diff --git a/Xi/xiproperty.c b/Xi/xiproperty.c
index 9acb2ebe3..6ec419e87 100644
--- a/Xi/xiproperty.c
+++ b/Xi/xiproperty.c
@@ -372,8 +372,7 @@ XIGetKnownProperty(const

[PATCH xserver 3/4] test: signal-logging: Fix looping signed number tests

2017-10-27 Thread Daniel Martin

unsigned_tests[] was used to compute the amount of signed numbers to
test.

Signed-off-by: Daniel Martin 
---
 test/signal-logging.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/signal-logging.c b/test/signal-logging.c
index 9bf39e58d..ca327e9aa 100644
--- a/test/signal-logging.c
+++ b/test/signal-logging.c
@@ -148,7 +148,7 @@ number_formatting(void)
 for (i = 0; i < sizeof(unsigned_tests) / sizeof(unsigned_tests[0]); i++)
 assert(check_number_format_test(unsigned_tests[i]));
 
-for (i = 0; i < sizeof(unsigned_tests) / sizeof(signed_tests[0]); i++)
+for (i = 0; i < sizeof(signed_tests) / sizeof(signed_tests[0]); i++)
 assert(check_signed_number_format_test(signed_tests[i]));
 
 for (i = 0; i < sizeof(float_tests) / sizeof(float_tests[0]); i++)
-- 
2.13.6

___
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 2/4] test: input: Fix used uninitialized warning in dix_event_to_core

2017-10-27 Thread Daniel Martin

input.c: In function ‘dix_event_to_core’:
../include/inputstr.h:61:55: warning: ‘*((void *)+80)’ is used uninitialized 
in this function [-Wuninitialized]
 #define SetBit(ptr, bit)  (((BYTE *) (ptr))[(bit)>>3] |= (1 << ((bit) & 7)))
   ^~

Signed-off-by: Daniel Martin 
---
 test/input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/input.c b/test/input.c
index 4cd39bb48..8638f1443 100644
--- a/test/input.c
+++ b/test/input.c
@@ -230,7 +230,7 @@ dix_check_grab_values(void)
 static void
 dix_event_to_core(int type)
 {
-DeviceEvent ev;
+DeviceEvent ev = {};
 xEvent *core;
 int time;
 int x, y;
-- 
2.13.6

___
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] os/inputthread: Force unlock when stopping thread.

2017-10-27 Thread Michal Srb

The inputthread is kept locked all the time while X server's VT is not active.
If the X server is terminated while not active, it will be stuck forever in
InputThreadFini waiting for the thread to join, but it wouldn't because it is
locked.
---
This fixes the X server termination while its VT is not active. I think it
should be safe to force unlock it from InputThreadFini - X server is
terminating and nothing else should race there, but I am not completely sure. 

 os/inputthread.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/os/inputthread.c b/os/inputthread.c
index 721e86312..dc4eb9f20 100644
--- a/os/inputthread.c
+++ b/os/inputthread.c
@@ -497,6 +497,7 @@ InputThreadFini(void)
 
 /* Close the pipe to get the input thread to shut down */
 close(hotplugPipeWrite);
+input_force_unlock();
 pthread_join(inputThreadInfo->thread, NULL);
 
 xorg_list_for_each_entry_safe(dev, next, >devs, node) {
-- 
2.12.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: Xwayland fatal error when Wayland output disappears

2017-10-27 Thread Pekka Paalanen

On Fri, 27 Oct 2017 09:54:39 +0200
Michael Thayer  wrote:

> Hello Pekka,
> 
> On 27.10.2017 08:01, Pekka Paalanen wrote:
> > On Thu, 26 Oct 2017 15:40:13 +0200 Michael Thayer
> >  wrote:  
> [ Discussion of a global Wayland object disappearing triggering a fatal
> error in xwl_log_handler in Xwayland.]
> > 
> > there is a known race around Wayland globals. If the Wayland server 
> > adds and removes a global in a very short time, it may succeed to 
> > remove the global (wl_output) before all clients have processed the 
> > add. If a client process an add after the server removed, you hit 
> > exactly this fatal error.
> > 
> > It's a design flaw in Wayland, gone unnoticed for years until it was 
> > too late to fix properly.
> > 
> > This issue is recorded: https://phabricator.freedesktop.org/T7722
> > 
> > There is a suggested mitigation, but I am not aware of anyone
> > working on it.  
> 
> Trigger warning: a person who does not know the code base is about to 
> make suggestions.
> 
> I am wondering why Xwayland has to call FatalError in that log handler.

Because the Wayland connection is already dead and disconnected due to
the error it is reporting.


Thanks,
pq

>  From my naive point of view, the race you mentioned is not a design 
> flaw in Wayland but part of the real world that Wayland and its clients 
> ought to deal with.  Other than that call in the log handler, I see no 
> reason why Xwayland should not just notice that the object has gone away 
> again and get on with life.
> 
> As I said, I do not know the code base well.  But for the sake of the 
> argument, and since it is not likely to make things worse, I will try 
> removing that locally and see what happens.
> 
> Regards and thanks.
> Michael



pgpqLYN4WaAJ6.pgp
Description: OpenPGP digital signature
___
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: Xwayland fatal error when Wayland output disappears

2017-10-27 Thread Michael Thayer


Hello Pekka,

On 27.10.2017 08:01, Pekka Paalanen wrote:

On Thu, 26 Oct 2017 15:40:13 +0200 Michael Thayer
 wrote:

[ Discussion of a global Wayland object disappearing triggering a fatal
error in xwl_log_handler in Xwayland.]


there is a known race around Wayland globals. If the Wayland server 
adds and removes a global in a very short time, it may succeed to 
remove the global (wl_output) before all clients have processed the 
add. If a client process an add after the server removed, you hit 
exactly this fatal error.


It's a design flaw in Wayland, gone unnoticed for years until it was 
too late to fix properly.


This issue is recorded: https://phabricator.freedesktop.org/T7722

There is a suggested mitigation, but I am not aware of anyone
working on it.


Trigger warning: a person who does not know the code base is about to 
make suggestions.


I am wondering why Xwayland has to call FatalError in that log handler. 
From my naive point of view, the race you mentioned is not a design 
flaw in Wayland but part of the real world that Wayland and its clients 
ought to deal with.  Other than that call in the log handler, I see no 
reason why Xwayland should not just notice that the object has gone away 
again and get on with life.


As I said, I do not know the code base well.  But for the sake of the 
argument, and since it is not likely to make things worse, I will try 
removing that locally and see what happens.


Regards and thanks.
Michael
--
Michael Thayer | VirtualBox engineer
ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstraße 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister
der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
___
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: Xwayland fatal error when Wayland output disappears

2017-10-27 Thread Pekka Paalanen

On Thu, 26 Oct 2017 15:40:13 +0200
Michael Thayer  wrote:

> Hello,
> 
> Reporting this here in case it is of interest.  I have been debugging
> regular desktop crashes on my new Ubuntu 17.10 systems.  One of them
> seems to happen when Xwayland FatalError-s out because a Wayland output
> disappears.  I have only investigated this so far, but hopefully this
> will be helpful to someone who knows it better: the error is
> "wl_registry@2: error 0: invalid global wl_output (28)", which seems to
> be posted by wl_registry_bind in wayland-server.c, and get picked up in
> xwl_log_handler in Xwayland.  Xwayland exiting in turn causes GNOME
> Shell to panic.

Hi,

there is a known race around Wayland globals. If the Wayland server
adds and removes a global in a very short time, it may succeed to
remove the global (wl_output) before all clients have processed the
add. If a client process an add after the server removed, you hit
exactly this fatal error.

It's a design flaw in Wayland, gone unnoticed for years until it was
too late to fix properly.

This issue is recorded: https://phabricator.freedesktop.org/T7722

There is a suggested mitigation, but I am not aware of anyone working
on it.

Thanks,
pq

pgpsQhLjZZMU_.pgp
Description: OpenPGP digital signature
___
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] present: Only send PresentCompleteNotify events to the presenting client

Re: [PATCH xserver] present: Only send PresentCompleteNotify events to the presenting client

Re: [PATCH xserver] xf86-video-modesetting: Fix ms_queue_vblank(flags = MS_QUEUE_RELATIVE)

[PATCH xserver 1/4] modesetting: Fix potential buffer overflow

[PATCH xserver 4/4] Use ARRAY_SIZE all over the tree

[PATCH xserver 3/4] test: signal-logging: Fix looping signed number tests

[PATCH xserver 2/4] test: input: Fix used uninitialized warning in dix_event_to_core

[PATCH] os/inputthread: Force unlock when stopping thread.

Re: Xwayland fatal error when Wayland output disappears

Re: Xwayland fatal error when Wayland output disappears

Re: Xwayland fatal error when Wayland output disappears

11 matches

Site Navigation

Mail list logo

Footer information