[prev in list] [next in list] [prev in thread] [next in thread]
List: freedesktop-xorg-devel
Subject: Re: [PATCH] Also dump passive grabs on XF86LogGrabInfo
From: Michael Stapelberg <stapelberg () google ! com>
Date: 2015-10-23 11:31:06
Message-ID: CAH9Oa-bPS3v49uE5sgpdJE_HVN4QOQqxzigSSXyFpf5e5GJ=nQ () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
I'll send the v2 patch in a second.
On Sun, Oct 18, 2015 at 8:53 PM, Peter Hutterer <peter.hutterer@who-t.net>
wrote:
> On Thu, Oct 15, 2015 at 09:32:56AM -0700, Michael Stapelberg wrote:
> > This patch is not entirely finished, I've marked the places that need
> further
> > attention with TODO. Any input on these, and on the patch in general, is
> > appreciated.
> >
> > Listing passive grabs is useful for debugging the case where users don't
> know
> > why a keybinding is not consumed by a particular application (typically
> because
> > another application is grabbing the binding).
> >
> > I've tried to be consistent in the messages with other places that dump
> grabs
> > or clients.
> >
> > ---
> > dix/window.c | 108
> ++++++++++++++++++++++++++++++++++++++++
> > hw/xfree86/dixmods/xkbPrivate.c | 2 +
> > include/window.h | 1 +
> > 3 files changed, 111 insertions(+)
> >
> > diff --git a/dix/window.c b/dix/window.c
> > index d57f320..5e15a72 100644
> > --- a/dix/window.c
> > +++ b/dix/window.c
> > @@ -127,6 +127,7 @@ Equipment Corporation.
> > #include "compint.h"
> > #endif
> > #include "selection.h"
> > +#include "inpututils.h"
> >
> > #include "privates.h"
> > #include "xace.h"
> > @@ -273,6 +274,113 @@ log_window_info(WindowPtr pWin, int depth)
> > ErrorF("\n");
> > }
> >
> > +static void
> > +log_grab_info(void *value, XID id, void *cdata)
> > +{
> > + GrabPtr pGrab;
> > + int i, j;
> > + pGrab = (GrabPtr)value;
> > +
> > + ErrorF(" grab 0x%lx (%s), type '%s' on window 0x%lx\n",
> > + (unsigned long) pGrab->resource,
> > + (pGrab->grabtype == XI2) ? "xi2" :
> > + ((pGrab->grabtype == CORE) ? "core" : "xi1"),
> > + pGrab->type == ButtonPress ? "ButtonPress" :
> > + // TODO: where do DeviceButtonPress and DeviceKeyPress come
> from?
> > + // They are undefined when compiling.
> > + //pGrab->type == DeviceButtonPress ? "DeviceButtonPress" :
>
> internally the server uses InternalEvents. depending on which client we
> send
> the event to, we convert that into a XI2, XI1 or core event. so a grab
> that's grabtype XI2 and type ButtonPress is a DeviceButtonPress grab.
> IOW, you can skip the conditions and just check the core types here.
>
Thanks for clarifying, I've fixed that.
>
> > + pGrab->type == XI_ButtonPress ? "XI_ButtonPress" :
> > + pGrab->type == KeyPress ? "KeyPress" :
> > + //pGrab->type == DeviceKeyPress ? "DeviceKeyPress" :
> > + "XI_KeyPress",
> > + (unsigned long) pGrab->window->drawable.id,
> > + pGrab->device->id);
> > + ErrorF(" detail %d (mask %lu), modifiersDetail %d (mask %lu)\n",
> > + pGrab->detail.exact,
> > + pGrab->detail.pMask ? (unsigned long) *(pGrab->detail.pMask)
> : 0,
> > + pGrab->modifiersDetail.exact,
> > + pGrab->modifiersDetail.pMask ?
> > + (unsigned long) *(pGrab->modifiersDetail.pMask) :
> > + (unsigned long) 0);
> > + ErrorF(" device '%s' (%d), modifierDevice '%s' (%d)\n",
> > + pGrab->device->name, pGrab->device->id,
> > + pGrab->modifierDevice->name, pGrab->modifierDevice->id);
> > + if (pGrab->grabtype == CORE) {
> > + ErrorF(" core event mask 0x%lx\n",
> > + (unsigned long) pGrab->eventMask);
> > + }
> > + else if (pGrab->grabtype == XI) {
> > + ErrorF(" xi1 event mask 0x%lx\n",
> > + (unsigned long) pGrab->eventMask);
> > + }
> > + else if (pGrab->grabtype == XI2) {
> > + for (i = 0; i < xi2mask_num_masks(pGrab->xi2mask); i++) {
> > + const unsigned char *mask;
> > + int print;
> > +
> > + print = 0;
> > + for (j = 0; j < XI2MASKSIZE; j++) {
> > + mask = xi2mask_get_one_mask(pGrab->xi2mask, i);
> > + if (mask[j]) {
> > + print = 1;
> > + break;
> > + }
> > + }
> > + if (!print)
> > + continue;
> > + ErrorF(" xi2 event mask 0x");
> > + for (j = 0; j < xi2mask_mask_size(pGrab->xi2mask); j++)
> > + ErrorF("%x", mask[j]);
>
> probably better to print this with spaces between each, it's hard to read
> otherwise.
>
Done.
>
> > + ErrorF("\n");
> > + }
> > + }
> > + ErrorF(" owner-events %s, kb %d ptr %d, confine 0x%lx, cursor
> 0x%lx\n",
> > + pGrab->ownerEvents ? "true" : "false",
> > + pGrab->keyboardMode, pGrab->pointerMode,
> > + pGrab->confineTo ? (unsigned long) pGrab->confineTo->
> drawable.id : 0,
> > + pGrab->cursor ? (unsigned long) pGrab->cursor->id : 0);
> > +
> > + // TODO: dump another grab if there is pGrab->next
>
> split the print into a helper function that takes a pGrab as argument, then
> looping through is easy.
>
Actually, turns out I misunderstood how this works: the next pointer is
used to build up a linked list. Since the code is already iterating over
the list, we don't need to follow the next pointer.
>
> Cheers,
> Peter
>
> > +}
> > +
> > +void
> > +PrintPassiveGrabs(void)
> > +{
> > + int i;
> > + LocalClientCredRec *lcc;
> > + pid_t clientpid;
> > + const char *cmdname;
> > + const char *cmdargs;
> > +
> > + ErrorF("Printing all currently registered grabs\n");
> > +
> > + for (i = 1; i < currentMaxClients; i++) {
> > + if (!clients[i] || clients[i]->clientState !=
> ClientStateRunning)
> > + continue;
> > +
> > + clientpid = GetClientPid(clients[i]);
> > + cmdname = GetClientCmdName(clients[i]);
> > + cmdargs = GetClientCmdArgs(clients[i]);
> > + if ((clientpid > 0) && (cmdname != NULL)) {
> > + ErrorF(" Printing all registered grabs of client pid %ld
> %s %s\n",
> > + (long) clientpid, cmdname, cmdargs ? cmdargs : "");
> > + } else {
> > + if (GetLocalClientCreds(clients[i], &lcc) == -1) {
> > + ErrorF(" GetLocalClientCreds() failed\n");
> > + continue;
> > + }
> > + ErrorF(" Printing all registered grabs of client pid %ld
> uid %ld gid %ld\n",
> > + (lcc->fieldsSet & LCC_PID_SET) ? (long) lcc->pid : 0,
> > + (lcc->fieldsSet & LCC_UID_SET) ? (long) lcc->euid :
> 0,
> > + (lcc->fieldsSet & LCC_GID_SET) ? (long) lcc->egid :
> 0);
> > + FreeLocalClientCreds(lcc);
> > + }
> > +
> > + FindClientResourcesByType(clients[i], RT_PASSIVEGRAB,
> log_grab_info, NULL);
> > + }
> > + ErrorF("End list of registered passive grabs\n");
> > +}
> > +
> > void
> > PrintWindowTree(void)
> > {
> > diff --git a/hw/xfree86/dixmods/xkbPrivate.c
> b/hw/xfree86/dixmods/xkbPrivate.c
> > index 574590f..4b9ef33 100644
> > --- a/hw/xfree86/dixmods/xkbPrivate.c
> > +++ b/hw/xfree86/dixmods/xkbPrivate.c
> > @@ -38,6 +38,8 @@ XkbDDXPrivate(DeviceIntPtr dev, KeyCode key, XkbAction
> *act)
> > if (tmp->deviceGrab.grab)
> > PrintDeviceGrabInfo(tmp);
> > xf86Msg(X_INFO, "End list of active device grabs\n");
> > +
> > + PrintPassiveGrabs();
> > }
> > else if (strcasecmp(msgbuf, "ungrab") == 0)
> > UngrabAllDevices(FALSE);
> > diff --git a/include/window.h b/include/window.h
> > index 6daec85..f13ed51 100644
> > --- a/include/window.h
> > +++ b/include/window.h
> > @@ -223,6 +223,7 @@ extern _X_EXPORT RegionPtr CreateClipShape(WindowPtr
> /* pWin */ );
> >
> > extern _X_EXPORT void SetRootClip(ScreenPtr pScreen, Bool enable);
> > extern _X_EXPORT void PrintWindowTree(void);
> > +extern _X_EXPORT void PrintPassiveGrabs(void);
> >
> > extern _X_EXPORT VisualPtr WindowGetVisual(WindowPtr /*pWin*/);
> > #endif /* WINDOW_H */
> > --
> > 2.1.4
> >
>
--
Best regards,
Michael
[Attachment #5 (text/html)]
<div dir="ltr">I'll send the v2 patch in a second.<br><div \
class="gmail_extra"><br><div class="gmail_quote">On Sun, Oct 18, 2015 at 8:53 PM, \
Peter Hutterer <span dir="ltr"><<a href="mailto:peter.hutterer@who-t.net" \
target="_blank">peter.hutterer@who-t.net</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu, Oct 15, 2015 at \
09:32:56AM -0700, Michael Stapelberg wrote:<br> > This patch is not entirely \
finished, I've marked the places that need further<br> > attention with TODO. Any \
input on these, and on the patch in general, is<br> > appreciated.<br>
><br>
> Listing passive grabs is useful for debugging the case where users don't \
know<br> > why a keybinding is not consumed by a particular application (typically \
because<br> > another application is grabbing the binding).<br>
><br>
> I've tried to be consistent in the messages with other places that dump \
grabs<br> > or clients.<br>
><br>
> ---<br>
> dix/window.c | 108 \
++++++++++++++++++++++++++++++++++++++++<br> > hw/xfree86/dixmods/xkbPrivate.c | \
2 +<br> > include/window.h | 1 +<br>
> 3 files changed, 111 insertions(+)<br>
><br>
> diff --git a/dix/window.c b/dix/window.c<br>
> index d57f320..5e15a72 100644<br>
> --- a/dix/window.c<br>
> +++ b/dix/window.c<br>
> @@ -127,6 +127,7 @@ Equipment Corporation.<br>
> #include "compint.h"<br>
> #endif<br>
> #include "selection.h"<br>
> +#include "inpututils.h"<br>
><br>
> #include "privates.h"<br>
> #include "xace.h"<br>
> @@ -273,6 +274,113 @@ log_window_info(WindowPtr pWin, int depth)<br>
> ErrorF("\n");<br>
> }<br>
><br>
> +static void<br>
> +log_grab_info(void *value, XID id, void *cdata)<br>
> +{<br>
> + GrabPtr pGrab;<br>
> + int i, j;<br>
> + pGrab = (GrabPtr)value;<br>
> +<br>
> + ErrorF(" grab 0x%lx (%s), type '%s' on window \
0x%lx\n",<br> > + (unsigned long) pGrab->resource,<br>
> + (pGrab->grabtype == XI2) ? "xi2" :<br>
> + ((pGrab->grabtype == CORE) ? "core" : \
"xi1"),<br> > + pGrab->type == ButtonPress ? \
"ButtonPress" :<br> > + // TODO: where do \
DeviceButtonPress and DeviceKeyPress come from?<br> > + // They \
are undefined when compiling.<br> > + //pGrab->type == \
DeviceButtonPress ? "DeviceButtonPress" :<br> <br>
</div></div>internally the server uses InternalEvents. depending on which client we \
send<br> the event to, we convert that into a XI2, XI1 or core event. so a grab<br>
that's grabtype XI2 and type ButtonPress is a DeviceButtonPress grab.<br>
IOW, you can skip the conditions and just check the core types \
here.<br></blockquote><div><br></div><div>Thanks for clarifying, I've fixed \
that.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"> <div><div class="h5"><br>
> + pGrab->type == XI_ButtonPress ? "XI_ButtonPress" \
:<br> > + pGrab->type == KeyPress ? "KeyPress" :<br>
> + //pGrab->type == DeviceKeyPress ? \
"DeviceKeyPress" :<br> > + "XI_KeyPress",<br>
> + (unsigned long) pGrab->window-><a \
href="http://drawable.id" rel="noreferrer" target="_blank">drawable.id</a>,<br> > \
+ pGrab->device->id);<br> > + ErrorF(" detail \
%d (mask %lu), modifiersDetail %d (mask %lu)\n",<br> > + \
pGrab->detail.exact,<br> > + pGrab->detail.pMask ? (unsigned \
long) *(pGrab->detail.pMask) : 0,<br> > + \
pGrab->modifiersDetail.exact,<br> > + \
pGrab->modifiersDetail.pMask ?<br> > + (unsigned long) \
*(pGrab->modifiersDetail.pMask) :<br> > + (unsigned long) \
0);<br> > + ErrorF(" device '%s' (%d), modifierDevice \
'%s' (%d)\n",<br> > + pGrab->device->name, \
pGrab->device->id,<br> > + \
pGrab->modifierDevice->name, pGrab->modifierDevice->id);<br> > + \
if (pGrab->grabtype == CORE) {<br> > + ErrorF(" core event \
mask 0x%lx\n",<br> > + (unsigned long) \
pGrab->eventMask);<br> > + }<br>
> + else if (pGrab->grabtype == XI) {<br>
> + ErrorF(" xi1 event mask 0x%lx\n",<br>
> + (unsigned long) pGrab->eventMask);<br>
> + }<br>
> + else if (pGrab->grabtype == XI2) {<br>
> + for (i = 0; i < xi2mask_num_masks(pGrab->xi2mask); i++) {<br>
> + const unsigned char *mask;<br>
> + int print;<br>
> +<br>
> + print = 0;<br>
> + for (j = 0; j < XI2MASKSIZE; j++) {<br>
> + mask = xi2mask_get_one_mask(pGrab->xi2mask, i);<br>
> + if (mask[j]) {<br>
> + print = 1;<br>
> + break;<br>
> + }<br>
> + }<br>
> + if (!print)<br>
> + continue;<br>
> + ErrorF(" xi2 event mask 0x");<br>
> + for (j = 0; j < xi2mask_mask_size(pGrab->xi2mask); \
j++)<br> > + ErrorF("%x", mask[j]);<br>
<br>
</div></div>probably better to print this with spaces between each, it's hard to \
read<br> otherwise.<br></blockquote><div><br></div><div>Done.</div><div> \
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <span class=""><br>
> + ErrorF("\n");<br>
> + }<br>
> + }<br>
> + ErrorF(" owner-events %s, kb %d ptr %d, confine 0x%lx, cursor \
0x%lx\n",<br> > + pGrab->ownerEvents ? "true" : \
"false",<br> > + pGrab->keyboardMode, \
pGrab->pointerMode,<br> > + pGrab->confineTo ? (unsigned \
long) pGrab->confineTo-><a href="http://drawable.id" rel="noreferrer" \
target="_blank">drawable.id</a> : 0,<br> > + pGrab->cursor ? \
(unsigned long) pGrab->cursor->id : 0);<br> > +<br>
> + // TODO: dump another grab if there is pGrab->next<br>
<br>
</span>split the print into a helper function that takes a pGrab as argument, \
then<br> looping through is easy.<br></blockquote><div><br></div><div>Actually, turns \
out I misunderstood how this works: the next pointer is used to build up a linked \
list. Since the code is already iterating over the list, we don't need to follow the \
next pointer.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"> <br>
Cheers,<br>
Peter<br>
<div class="HOEnZb"><div class="h5"><br>
> +}<br>
> +<br>
> +void<br>
> +PrintPassiveGrabs(void)<br>
> +{<br>
> + int i;<br>
> + LocalClientCredRec *lcc;<br>
> + pid_t clientpid;<br>
> + const char *cmdname;<br>
> + const char *cmdargs;<br>
> +<br>
> + ErrorF("Printing all currently registered grabs\n");<br>
> +<br>
> + for (i = 1; i < currentMaxClients; i++) {<br>
> + if (!clients[i] || clients[i]->clientState != \
ClientStateRunning)<br> > + continue;<br>
> +<br>
> + clientpid = GetClientPid(clients[i]);<br>
> + cmdname = GetClientCmdName(clients[i]);<br>
> + cmdargs = GetClientCmdArgs(clients[i]);<br>
> + if ((clientpid > 0) && (cmdname != NULL)) {<br>
> + ErrorF(" Printing all registered grabs of client pid \
%ld %s %s\n",<br> > + (long) clientpid, cmdname, \
cmdargs ? cmdargs : "");<br> > + } else {<br>
> + if (GetLocalClientCreds(clients[i], &lcc) == -1) {<br>
> + ErrorF(" GetLocalClientCreds() \
failed\n");<br> > + continue;<br>
> + }<br>
> + ErrorF(" Printing all registered grabs of client pid \
%ld uid %ld gid %ld\n",<br> > + \
(lcc->fieldsSet & LCC_PID_SET) ? (long) lcc->pid : 0,<br> > + \
(lcc->fieldsSet & LCC_UID_SET) ? (long) lcc->euid : 0,<br> > + \
(lcc->fieldsSet & LCC_GID_SET) ? (long) lcc->egid : 0);<br> > + \
FreeLocalClientCreds(lcc);<br> > + }<br>
> +<br>
> + FindClientResourcesByType(clients[i], RT_PASSIVEGRAB, \
log_grab_info, NULL);<br> > + }<br>
> + ErrorF("End list of registered passive grabs\n");<br>
> +}<br>
> +<br>
> void<br>
> PrintWindowTree(void)<br>
> {<br>
> diff --git a/hw/xfree86/dixmods/xkbPrivate.c \
b/hw/xfree86/dixmods/xkbPrivate.c<br> > index 574590f..4b9ef33 100644<br>
> --- a/hw/xfree86/dixmods/xkbPrivate.c<br>
> +++ b/hw/xfree86/dixmods/xkbPrivate.c<br>
> @@ -38,6 +38,8 @@ XkbDDXPrivate(DeviceIntPtr dev, KeyCode key, XkbAction \
*act)<br> > if (tmp->deviceGrab.grab)<br>
> PrintDeviceGrabInfo(tmp);<br>
> xf86Msg(X_INFO, "End list of active device \
grabs\n");<br> > +<br>
> + PrintPassiveGrabs();<br>
> }<br>
> else if (strcasecmp(msgbuf, "ungrab") == 0)<br>
> UngrabAllDevices(FALSE);<br>
> diff --git a/include/window.h b/include/window.h<br>
> index 6daec85..f13ed51 100644<br>
> --- a/include/window.h<br>
> +++ b/include/window.h<br>
> @@ -223,6 +223,7 @@ extern _X_EXPORT RegionPtr CreateClipShape(WindowPtr /* pWin \
*/ );<br> ><br>
> extern _X_EXPORT void SetRootClip(ScreenPtr pScreen, Bool enable);<br>
> extern _X_EXPORT void PrintWindowTree(void);<br>
> +extern _X_EXPORT void PrintPassiveGrabs(void);<br>
><br>
> extern _X_EXPORT VisualPtr WindowGetVisual(WindowPtr /*pWin*/);<br>
> #endif /* WINDOW_H */<br>
> --<br>
> 2.1.4<br>
><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div \
class="gmail_signature">Best regards,<div>Michael</div></div> </div></div>
[Attachment #6 (text/plain)]
_______________________________________________
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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic