if I try
$ xwininfo -name ZZZ
where ZZZ is some name for which no window exists, I get a wrong window, one which happens to belong to a Firefox 4 I am running. Clearly, this window's name *isn't* ZZZ - rather it has no name:
$ xwininfo -name ZZZ
xwininfo: Window id: 0x2a0006d (has no name)
Absolute upper-left X: 0
Absolute upper-left Y: 0
Relative upper-left X: 0
Relative upper-left Y: 0
I get the same window regardless of which non-existant name I use. I never get the error message I should have gotten in this case.
Looking at the code, in dsimple.c's Window_With_Name(), I cannot find an obvious reason for this bug. However, I think I found another unrelated oversight there: You use XFetchName, but forget to XFree window_name after you used it.
By the way, I'm using Fedora 15 and xorg-x11-utils-7.5-2.fc15.x86_64
I just compiled the latest xwininfo myself (http://www.x.org/releases/X11R7.6/src/app/xwininfo-1.1.1.tar.gz) and the bug remains there.
It appears that the code I previously looked at wasn't the latest code - the latest code appears to be using XCB instead of XLIB - I didn't know that. It is sad for me to see how this simple application where speed is of no importance became so convoluted by using XCB :( It has been so long since I last read X sources, that I didn't even know this was happening.
Anyway, the bug in the new code I found now is pretty obvious:
prop_name_len = xcb_get_property_value_length (prop);
strncmp (prop_name, name, prop_name_len)
is wrong! If prop_name is "", it checks zero characters, and obviously finds a match ;-) It should have used name's length, not prop_name's length!
There are two other instances of this wrong check in the code.
Using strlen(name) in all calls of strncmp in dsimple.c fixed this bug.
Please apply this fix.
On second thought, using strlen(name), while better, is also not accurate - it can cause "xwininfo -name hell" to find a window named "hello" :-)
We need a new comparison function which
1. First compares the lengths,
2. And only if the lengths are equal, call strncmp.
Again, this is a very simple fix, and quite important: I was using "xwininfo -name ..." to find windows with a certain name and kill them, and then, after upgrading my system, the script which used to work well started to kill my Firefox windows :(
(In reply to comment #1)
> Please apply this fix.
Could you create a patch, please?
Created attachment 47314 [details] [review]
Fixes the bug described above.
Here is my proposed fix for the bug: In addition to strncmp, we also need to check that the lengths are identical - otherwise any string matches with an empty window name :-(
(In reply to comment #4)
> Created an attachment (id=47314) [details]
> Fixes the bug described above.
> Here is my proposed fix for the bug: In addition to strncmp, we also need to
> check that the lengths are identical - otherwise any string matches with an
> empty window name :-(
I am sorry, I was reprimanded that xorg doesn't use patches-as-attachments to bugs, but you have to follow http://wiki.x.org/wiki/Development/Documentation/SubmittingPatches.
I am sorry for the confusion again.
The well known saying is that you can lead a horse to water, but you cannot make it drink.
In this case, I found a rather serious regression in the "xwininfo" tool, explained where the bug is, and even wrote code to fix it. I think that for anyone that cares about this code, this is more than enough, even if it doesn't follow the accurate patch submission instructions, whatever these are... After all, this is a bug report, not a new feature request.
When I tried to understand why xwininfo, a decades-old tool, is suddenly written in XCB and not Xlib, I found that (see http://lists.freedesktop.org/archives/xcb/2010-June/006111.html), the author "mostly did this as a way to learn more xcb". I hope he continues to care about the code, and improve his learning :-)
(In reply to comment #6)
> The well known saying is that you can lead a horse to water, but you cannot
> make it drink.
Sure, of course, I (or more likely Alan) can send that email to the list myself, I just didn't want to steal fame and ohloh.net points from you.
Adding Alan to CC of this bug.
Thanks for the cc, but I get xorg-team mail, so already got all of it.
Patches to the mailing list are preferred but I can easily take the patch
from bugzilla & apply it. Thanks for doing the work to diagnose this.
While I did the port to xcb so I could learn it better, that's not the
only reason for having it ported - it does improve performance over low
latency and helps serve as a xcb test case and example.
Patch proposing a similar fix to the one in attachment 47314 [details] [review] submitted
to xorg-devel for review:
Fix pushed to git master: