From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: x-display-pixel-width/height inconsistency Date: Fri, 10 May 2013 10:06:18 +0300 Message-ID: <837gj7co0l.fsf@gnu.org> References: <514A5DE1.10009@gmx.de> <831ub767wf.fsf@gnu.org> <83mwtu4p7c.fsf@gnu.org> <83vc8h313t.fsf@gnu.org> <5073D6B8-95E4-4012-AA74-106F428379DC@swipnet.se> <8BD4B041-5A3F-4D7C-AFD3-E997E194AA9D@swipnet.se> <02B98FCD-71DB-47EC-B58B-41A2539FF61A@swipnet.se> <83vc6tcqss.fsf@gnu.org> <83haibdipo.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1368170013 9714 80.91.229.3 (10 May 2013 07:13:33 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 10 May 2013 07:13:33 +0000 (UTC) Cc: emacs-devel@gnu.org To: YAMAMOTO Mitsuharu Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri May 10 09:13:33 2013 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1UahWF-00051g-R8 for ged-emacs-devel@m.gmane.org; Fri, 10 May 2013 09:13:32 +0200 Original-Received: from localhost ([::1]:45998 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UahWF-0005s6-5D for ged-emacs-devel@m.gmane.org; Fri, 10 May 2013 03:13:31 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:40464) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UahW0-0005Zr-FB for emacs-devel@gnu.org; Fri, 10 May 2013 03:13:17 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UahVy-0003Is-KR for emacs-devel@gnu.org; Fri, 10 May 2013 03:13:16 -0400 Original-Received: from mtaout22.012.net.il ([80.179.55.172]:64814) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UahPV-0001on-ML for emacs-devel@gnu.org; Fri, 10 May 2013 03:06:34 -0400 Original-Received: from conversion-daemon.a-mtaout22.012.net.il by a-mtaout22.012.net.il (HyperSendmail v2007.08) id <0MMK00400MCHY000@a-mtaout22.012.net.il> for emacs-devel@gnu.org; Fri, 10 May 2013 10:06:32 +0300 (IDT) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout22.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0MMK004SQMEVWU20@a-mtaout22.012.net.il>; Fri, 10 May 2013 10:06:31 +0300 (IDT) In-reply-to: X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: Solaris 10 X-Received-From: 80.179.55.172 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:159466 Archived-At: > Date: Fri, 10 May 2013 15:00:33 +0900 > From: YAMAMOTO Mitsuharu > Cc: emacs-devel@gnu.org > > > It makes me nervous to have consing inside a Win32 API callback. Are > > we even sure the callback gets called in the context of our main > > tread? If not, and if consing causes GC, we could blow up the stack. > > If the callback can be called from the thread/context other than the > one for EnumDisplayMonitors and that can cause memory synchronization > issue, then API documentation should clearly say so, because such kind > of problem can happen even with other data structures. Well, we can sue MS, but that won't help us, I think. Seriously: while debugging Emacs on Windows, I frequently see messages from GDB about new threads being launched, which I know for sure Emacs didn't do directly. I have no idea why they are started. For example, calling GetOpenFileName certainly starts a new thread, but the MSDN documentation says nothing about that. (We've had problems with the stack size of that thread, see bug #13065.) We define a callback for that API, and I suspect that callback is run in the context of that additional thread. I don't know if the API you are using is different in this regard, but I prefer not to risk that, if we can avoid that risk. > And it is not Fcons but Feval that can cause GC. Thank you for your lecture. > > And actually, I don't see why you need the list of monitor HMONITOR > > handles, you get them from MonitorFromWindow anyway. All you need is > > the count of the monitors, and then you can create the vector in > > monitor_frames by simply inserting every monitor you get from > > MonitorFromWindow, and leave the rest at nil. Or am I missing > > something? > > > So could we please get rid of the consing inside the callback, and > > just count them instead? > > The display-monitor-function lists all the monitors regardless of the > existence of Emacs frames in it. I know that, but you end up with just nil in the list of frames for those monitors that don't have frames, right? So just use the callback to count the monitors; then you don't need to cons a list inside the callback, you only need to increment a counter. The cells for the monitors that don't have frames just need to be filled with nil. > >> + if (FRAME_W32_P (f) && FRAME_W32_DISPLAY_INFO (f) == dpyinfo > >> + && !EQ (frame, tip_frame)) > > > The test against dpyinfo is redundant, it will always be true: there's > > only one dpyinfo on Windows. I would like to remove it, because it's > > confusing. (There's one more such test in another place in the > > patch.) > > I'm aware of that. It's just for resemblance to X11 code. I'd rather > leave it to make it easier to compare with the code for X11 if it were > for the Mac port, but I don't have a strong opinion for W32. The w32 display doesn't support multiple display terminals, and probably never will. Even if it does some day support that, I doubt that the infrastructure for that will be similar to X. So please remove that. > >> + name = make_unibyte_string (mi.szDevice, strlen (mi.szDevice)); > > > Why make_unibyte_string? Is szDevice documented to be an ASCII-only > > string? > > make_unibyte_string does not assume that the passed bytes are only of > ASCII characters. Any other string will get users in trouble, because this will produce a unibyte string, something Lisp programs generally don't expect to get when the string is supposed to be human-readable text. > Of course, you can decode the string if its encoding is known (I'm not > sure how it is encoded because I'm not familiar with W32). You want DECODE_SYSTEM. It knows about the encoding of strings in the current session. > But you should be careful because code conversions can involve GC in > general unlike Fcons. Then use GCPRO (although I don't think it's needed in this case). It's not a big deal. > Alternatively, as the `name' attribute is optional, you can remove > it entirely if you don't like make_unibyte_string. I see no reason to remove it due to such a minor and easily solvable issue. Thanks.