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: Thu, 09 May 2013 23:03:15 +0300 Message-ID: <83haibdipo.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> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1368129809 7152 80.91.229.3 (9 May 2013 20:03:29 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 9 May 2013 20:03:29 +0000 (UTC) Cc: emacs-devel@gnu.org To: YAMAMOTO Mitsuharu Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu May 09 22:03:27 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 1UaX3n-0005zs-5c for ged-emacs-devel@m.gmane.org; Thu, 09 May 2013 22:03:27 +0200 Original-Received: from localhost ([::1]:56519 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UaX3m-0006Fa-Ph for ged-emacs-devel@m.gmane.org; Thu, 09 May 2013 16:03:26 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:43258) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UaX3h-0006FT-KV for emacs-devel@gnu.org; Thu, 09 May 2013 16:03:23 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UaX3f-0000lF-CK for emacs-devel@gnu.org; Thu, 09 May 2013 16:03:21 -0400 Original-Received: from mtaout23.012.net.il ([80.179.55.175]:48656) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UaX3f-0000l7-45 for emacs-devel@gnu.org; Thu, 09 May 2013 16:03:19 -0400 Original-Received: from conversion-daemon.a-mtaout23.012.net.il by a-mtaout23.012.net.il (HyperSendmail v2007.08) id <0MMJ00200RLC5Z00@a-mtaout23.012.net.il> for emacs-devel@gnu.org; Thu, 09 May 2013 23:03:17 +0300 (IDT) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout23.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0MMJ0020JRPF1W50@a-mtaout23.012.net.il>; Thu, 09 May 2013 23:03:17 +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.175 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:159455 Archived-At: > Date: Thu, 09 May 2013 09:09:33 +0900 > From: YAMAMOTO Mitsuharu > Cc: emacs-devel@gnu.org > > Thanks for testing. I updated the w32fns.c part below. I have a couple more comments. > + static BOOL CALLBACK > + w32_monitor_enum (HMONITOR monitor, HDC hdc, RECT *rcMonitor, LPARAM dwData) > + { > + Lisp_Object *monitor_list = (Lisp_Object *) dwData; > + > + *monitor_list = Fcons (make_save_pointer (monitor), *monitor_list); > + > + return TRUE; > + } 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. 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? > + 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.) > + name = make_unibyte_string (mi.szDevice, strlen (mi.szDevice)); Why make_unibyte_string? Is szDevice documented to be an ASCII-only string? > + attributes = Fcons (Fcons (Qname, build_string ("combined screen")), > + attributes); And using build_string here with a pure ASCII string is at least inconsistent. (But I actually suggest to use build_string in both cases.) Thanks.