Message ID | yddlgptxb7l.fsf@CeBiTec.Uni-Bielefeld.DE |
---|---|
State | New, archived |
Headers |
Received: (qmail 130063 invoked by alias); 19 May 2017 12:50:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 130033 invoked by uid 89); 19 May 2017 12:50:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-19.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=ao X-HELO: smtp.CeBiTec.Uni-Bielefeld.DE Received: from smtp.CeBiTec.Uni-Bielefeld.DE (HELO smtp.CeBiTec.Uni-Bielefeld.DE) (129.70.160.84) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 19 May 2017 12:50:07 +0000 Received: from localhost (localhost.CeBiTec.Uni-Bielefeld.DE [127.0.0.1]) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTP id 72AC6ED8; Fri, 19 May 2017 14:50:08 +0200 (CEST) Received: from smtp.CeBiTec.Uni-Bielefeld.DE ([127.0.0.1]) by localhost (malfoy.CeBiTec.Uni-Bielefeld.DE [127.0.0.1]) (amavisd-new, port 10024) with LMTP id n-DgN2sxwU14; Fri, 19 May 2017 14:50:06 +0200 (CEST) Received: from lokon.CeBiTec.Uni-Bielefeld.DE (lokon.CeBiTec.Uni-Bielefeld.DE [129.70.161.152]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTPS id 7F476ED5; Fri, 19 May 2017 14:50:06 +0200 (CEST) Received: (from ro@localhost) by lokon.CeBiTec.Uni-Bielefeld.DE (8.15.2+Sun/8.15.2/Submit) id v4JCo6wH027165; Fri, 19 May 2017 14:50:06 +0200 (MEST) From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> To: Pedro Alves <palves@redhat.com> Cc: gdb-patches@sourceware.org Subject: Re: Fix tui compilation with Solaris libcurses (PR tui/21482) References: <yddh90i1r31.fsf@CeBiTec.Uni-Bielefeld.DE> <32521e83-00b5-e2a8-faff-03b5407cfc67@redhat.com> <yddr2zmz3qc.fsf@CeBiTec.Uni-Bielefeld.DE> Date: Fri, 19 May 2017 14:50:06 +0200 In-Reply-To: <yddr2zmz3qc.fsf@CeBiTec.Uni-Bielefeld.DE> (Rainer Orth's message of "Thu, 18 May 2017 15:36:27 +0200") Message-ID: <yddlgptxb7l.fsf@CeBiTec.Uni-Bielefeld.DE> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (usg-unix-v) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" |
Commit Message
Rainer Orth
May 19, 2017, 12:50 p.m. UTC
Hi Pedro, >>> Ok for mainline and 8.0 branch? >> >> The cast bits are OK. I'd like to hear your opinion on >> moving the NOMACROS define to gdb_curses.h, before including >> <curses.h>. > > The move makes sense to me: I just wasn't aware of that file. I'll > prepare a separate patch. I've checked in the cast part now. Here's the NOMACROS part for gdb_curses.h. Tested as before on sparcv9-sun-solaris2.10 (curses) and amd64-pc-solaris2.12 (ncurses). Ok too? Interestingly, with that patch the previous link failure on Solaris 11/12 (missing wattr_on/wattr_off) is gone, too. This seems to happen because <ncurses/ncurses.h> no longer defines /usr/include/ncurses/ncurses.h:#define wattron(win,at) wattr_on(win, NCURSES_CAST(attr_t, at), NULL) /usr/include/ncurses/ncurses.h:#define attr_on(a,o) wattr_on(stdscr,a,o) and the references to wattron can be satisfied from libcurses just as from libncurses. Thanks. Rainer
Comments
On 05/19/2017 01:50 PM, Rainer Orth wrote: > Hi Pedro, > >>>> Ok for mainline and 8.0 branch? >>> >>> The cast bits are OK. I'd like to hear your opinion on >>> moving the NOMACROS define to gdb_curses.h, before including >>> <curses.h>. >> >> The move makes sense to me: I just wasn't aware of that file. I'll >> prepare a separate patch. > > I've checked in the cast part now. Here's the NOMACROS part for > gdb_curses.h. Tested as before on sparcv9-sun-solaris2.10 (curses) and > amd64-pc-solaris2.12 (ncurses). Ok too? OK. > > Interestingly, with that patch the previous link failure on Solaris > 11/12 (missing wattr_on/wattr_off) is gone, too. This seems to happen > because <ncurses/ncurses.h> no longer defines > > /usr/include/ncurses/ncurses.h:#define wattron(win,at) wattr_on(win, NCURSES_CAST(attr_t, at), NULL) > /usr/include/ncurses/ncurses.h:#define attr_on(a,o) wattr_on(stdscr,a,o) > > and the references to wattron can be satisfied from libcurses just as > from libncurses. Eh, great. Thanks, Pedro Alves
> From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> > Cc: gdb-patches@sourceware.org > Date: Fri, 19 May 2017 14:50:06 +0200 > > I've checked in the cast part now. Here's the NOMACROS part for > gdb_curses.h. Tested as before on sparcv9-sun-solaris2.10 (curses) and > amd64-pc-solaris2.12 (ncurses). Ok too? I think this should be guarded by some OS-specific macro, so as not to affect other platforms, where the original problem doesn't exist. (I see 6 instances of these macros being tested in my ncurses headers, and I'm not on Solaris.) Who knows what new problems this could cause? Thanks.
Hi Eli, >> I've checked in the cast part now. Here's the NOMACROS part for >> gdb_curses.h. Tested as before on sparcv9-sun-solaris2.10 (curses) and >> amd64-pc-solaris2.12 (ncurses). Ok too? > > I think this should be guarded by some OS-specific macro, so as not to > affect other platforms, where the original problem doesn't exist. (I > see 6 instances of these macros being tested in my ncurses headers, > and I'm not on Solaris.) Who knows what new problems this could cause? that's what I had done initially (via configure.ac for solaris2.* only), but Pedro suggested to do it unconditionally since some other targets (AIX notably) seem to be having the same problem. Rainer
On 05/19/2017 02:20 PM, Eli Zaretskii wrote: >> From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> >> Cc: gdb-patches@sourceware.org >> Date: Fri, 19 May 2017 14:50:06 +0200 >> >> I've checked in the cast part now. Here's the NOMACROS part for >> gdb_curses.h. Tested as before on sparcv9-sun-solaris2.10 (curses) and >> amd64-pc-solaris2.12 (ncurses). Ok too? > > I think this should be guarded by some OS-specific macro, so as not to > affect other platforms, where the original problem doesn't exist. The problem exists, we just hadn't tripped on it yet. Googling around we see people running into curses defining "move" as macro for instance, which conflicts with std::move. So I'd rather take the opposite approach. GDB must be able to compile with NOMACROS defined on some hosts, so defining it everywhere makes gdb behave the same everywhere and reduces the testing axis. A patch introducing a problem for Solaris or any host using the same curses will be quickly exposed on any host. > (I > see 6 instances of these macros being tested in my ncurses headers, > and I'm not on Solaris.) Who knows what new problems this could cause? I expect it'll fix more problems than create them. Defining macros without some sort of namespace prefix in C++ is a sure recipe for problems. NOMACROS was surely invented to avoid these problems. Thanks, Pedro Alves
On 05/19/2017 02:26 PM, Rainer Orth wrote: > Hi Eli, > >>> I've checked in the cast part now. Here's the NOMACROS part for >>> gdb_curses.h. Tested as before on sparcv9-sun-solaris2.10 (curses) and >>> amd64-pc-solaris2.12 (ncurses). Ok too? >> >> I think this should be guarded by some OS-specific macro, so as not to >> affect other platforms, where the original problem doesn't exist. (I >> see 6 instances of these macros being tested in my ncurses headers, >> and I'm not on Solaris.) Who knows what new problems this could cause? > > that's what I had done initially (via configure.ac for solaris2.* only), > but Pedro suggested to do it unconditionally since some other targets > (AIX notably) seem to be having the same problem. Yes, and it's not host specific, but really curses-implementation specific. On the same host you may compile against different versions of curses (BSD curses, ncurses, pdcurses, etc.). I don't see any benefit to complicate things when we have no evidence that telling curses to avoid defining its symbols as macros causes problems. Thanks, Pedro Alves
# HG changeset patch # Parent 32840d58190409875fc9d8590fcb97eafaaeeca3 Fix tui compilation with Solaris libcurses: clear define (PR tui/21482) diff --git a/gdb/gdb_curses.h b/gdb/gdb_curses.h --- a/gdb/gdb_curses.h +++ b/gdb/gdb_curses.h @@ -32,6 +32,13 @@ #undef KEY_EVENT #endif +/* On Solaris and probably other SysVr4 derived systems, we need to define + NOMACROS so the native <curses.h> doesn't define clear which interferes + with the clear member of class string_file. ncurses potentially has a + similar problem and fix. */ +#define NOMACROS +#define NCURSES_NOMACROS + #if defined (HAVE_NCURSES_NCURSES_H) #include <ncurses/ncurses.h> #elif defined (HAVE_NCURSES_H)