GNU bug report logs - #43195
[PATCH] Remove definitions of UP, BC and PC which should be provided by terminfo

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 43195 in the body.
You can then email your comments to 43195 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Fangrui Song <maskray <at> google.com>
To: bug-gnu-emacs <at> gnu.org
Cc: Fangrui Song <maskray <at> google.com>
Subject: [PATCH] Remove definitions of UP, BC and PC which should be provided
 by terminfo
Date: Thu,  3 Sep 2020 17:57:48 -0700

Otherwise if terminfo.c is compiled with -fno-common (GCC 10 and clang
11 default) and the archive version of the terminfo library is linked,
there will be a multiple definition linker error.

* src/terminfo.c (UP, BC, PC): Delete.
---
 src/terminfo.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/terminfo.c b/src/terminfo.c
index 51fd32e9e0..cc93d1012e 100644
--- a/src/terminfo.c
+++ b/src/terminfo.c
@@ -21,12 +21,6 @@
 
 #include "lisp.h"
 
-/* Define these variables that serve as global parameters to termcap,
-   so that we do not need to conditionalize the places in Emacs
-   that set them.  */
-
-char *UP, *BC, PC;
-
 /* Interface to curses/terminfo library.
    Turns out that all of the terminfo-level routines look
    like their termcap counterparts except for tparm, which replaces
-- 
2.28.0.526.ge36021eeef-goog

Message #8 received at 43195 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Fangrui Song <maskray <at> google.com>
Cc: 43195 <at> debbugs.gnu.org, maskray <at> google.com
Subject: Re: bug#43195: [PATCH] Remove definitions of UP,
 BC and PC which should be provided by terminfo
Date: Fri, 04 Sep 2020 10:36:45 +0300

> Cc: Fangrui Song <maskray <at> google.com>
> Date: Thu,  3 Sep 2020 17:57:48 -0700
> From: Fangrui Song via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> Otherwise if terminfo.c is compiled with -fno-common (GCC 10 and clang
> 11 default) and the archive version of the terminfo library is linked,
> there will be a multiple definition linker error.
> 
> * src/terminfo.c (UP, BC, PC): Delete.

Given the comment there, I think this should be conditioned on
actually using terminfo.  Does the following patch work for you?

diff --git a/src/terminfo.c b/src/terminfo.c
index 51fd32e..0765996 100644
--- a/src/terminfo.c
+++ b/src/terminfo.c
@@ -23,9 +23,12 @@
 
 /* Define these variables that serve as global parameters to termcap,
    so that we do not need to conditionalize the places in Emacs
-   that set them.  */
+   that set them.  But don't do that for terminfo, as that could
+   cause link errors when using -fno-common.  */
 
+#if !TERMINFO
 char *UP, *BC, PC;
+#endif
 
 /* Interface to curses/terminfo library.
    Turns out that all of the terminfo-level routines look

Message #11 received at 43195 <at> debbugs.gnu.org (full text, mbox):

From: Fangrui Song <maskray <at> google.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 43195 <at> debbugs.gnu.org
Subject: Re: bug#43195: [PATCH] Remove definitions of UP, BC and PC which
 should be provided by terminfo
Date: Fri, 4 Sep 2020 08:38:03 -0700

On 2020-09-04, Eli Zaretskii wrote:
>> Cc: Fangrui Song <maskray <at> google.com>
>> Date: Thu,  3 Sep 2020 17:57:48 -0700
>> From: Fangrui Song via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>
>> Otherwise if terminfo.c is compiled with -fno-common (GCC 10 and clang
>> 11 default) and the archive version of the terminfo library is linked,
>> there will be a multiple definition linker error.
>>
>> * src/terminfo.c (UP, BC, PC): Delete.
>
>Given the comment there, I think this should be conditioned on
>actually using terminfo.  Does the following patch work for you?
>
>diff --git a/src/terminfo.c b/src/terminfo.c
>index 51fd32e..0765996 100644
>--- a/src/terminfo.c
>+++ b/src/terminfo.c
>@@ -23,9 +23,12 @@
>
> /* Define these variables that serve as global parameters to termcap,
>    so that we do not need to conditionalize the places in Emacs
>-   that set them.  */
>+   that set them.  But don't do that for terminfo, as that could
>+   cause link errors when using -fno-common.  */
>
>+#if !TERMINFO
> char *UP, *BC, PC;
>+#endif
>
> /* Interface to curses/terminfo library.
>    Turns out that all of the terminfo-level routines look
>

Looks great! Thanks!

One nit, 

  #if !TERMINFO

probably should be 

  #ifndef TERMINFO


I don't know whether it is worth mentioning that -fcommon/-fno-common does not
cause a linker error when libtinfo.so is linked (a common/regular definition
preempts a shared definition).

-fno-common + libtinfo.a => multiple definition error

Message #14 received at 43195 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Fangrui Song <maskray <at> google.com>
Cc: 43195 <at> debbugs.gnu.org
Subject: Re: bug#43195: [PATCH] Remove definitions of UP, BC and PC which
 should be provided by terminfo
Date: Sat, 12 Sep 2020 10:19:48 +0300

> Date: Fri, 4 Sep 2020 08:38:03 -0700
> From: Fangrui Song <maskray <at> google.com>
> Cc: 43195 <at> debbugs.gnu.org
> 
> >diff --git a/src/terminfo.c b/src/terminfo.c
> >index 51fd32e..0765996 100644
> >--- a/src/terminfo.c
> >+++ b/src/terminfo.c
> >@@ -23,9 +23,12 @@
> >
> > /* Define these variables that serve as global parameters to termcap,
> >    so that we do not need to conditionalize the places in Emacs
> >-   that set them.  */
> >+   that set them.  But don't do that for terminfo, as that could
> >+   cause link errors when using -fno-common.  */
> >
> >+#if !TERMINFO
> > char *UP, *BC, PC;
> >+#endif
> >
> > /* Interface to curses/terminfo library.
> >    Turns out that all of the terminfo-level routines look
> >
> 
> Looks great! Thanks!

Thanks, installed for Emacs 27.2.

> One nit, 
> 
>    #if !TERMINFO
> 
> probably should be 
> 
>    #ifndef TERMINFO

That's the same thing with modern compilers.

> I don't know whether it is worth mentioning that -fcommon/-fno-common does not
> cause a linker error when libtinfo.so is linked (a common/regular definition
> preempts a shared definition).
> 
> -fno-common + libtinfo.a => multiple definition error

OK, but the change works even in these other cases, right?

Message #17 received at 43195 <at> debbugs.gnu.org (full text, mbox):

From: Fāng-ruì Sòng <maskray <at> google.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 43195 <at> debbugs.gnu.org
Subject: Re: bug#43195: [PATCH] Remove definitions of UP, BC and PC which
 should be provided by terminfo
Date: Sat, 12 Sep 2020 00:33:27 -0700

On Sat, Sep 12, 2020 at 12:19 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > Date: Fri, 4 Sep 2020 08:38:03 -0700
> > From: Fangrui Song <maskray <at> google.com>
> > Cc: 43195 <at> debbugs.gnu.org
> >
> > >diff --git a/src/terminfo.c b/src/terminfo.c
> > >index 51fd32e..0765996 100644
> > >--- a/src/terminfo.c
> > >+++ b/src/terminfo.c
> > >@@ -23,9 +23,12 @@
> > >
> > > /* Define these variables that serve as global parameters to termcap,
> > >    so that we do not need to conditionalize the places in Emacs
> > >-   that set them.  */
> > >+   that set them.  But don't do that for terminfo, as that could
> > >+   cause link errors when using -fno-common.  */
> > >
> > >+#if !TERMINFO
> > > char *UP, *BC, PC;
> > >+#endif
> > >
> > > /* Interface to curses/terminfo library.
> > >    Turns out that all of the terminfo-level routines look
> > >
> >
> > Looks great! Thanks!
>
> Thanks, installed for Emacs 27.2.
>
> > One nit,
> >
> >    #if !TERMINFO
> >
> > probably should be
> >
> >    #ifndef TERMINFO
>
> That's the same thing with modern compilers.
>
> > I don't know whether it is worth mentioning that -fcommon/-fno-common does not
> > cause a linker error when libtinfo.so is linked (a common/regular definition
> > preempts a shared definition).
> >
> > -fno-common + libtinfo.a => multiple definition error
>
> OK, but the change works even in these other cases, right?

Yep! The new code should work in other cases, -fcommon/-fno-common x
libtinfo.a/libtinfo.so

Message #22 received at 43195-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Fāng-ruì Sòng <maskray <at> google.com>
Cc: 43195-done <at> debbugs.gnu.org
Subject: Re: bug#43195: [PATCH] Remove definitions of UP, BC and PC which
 should be provided by terminfo
Date: Sat, 12 Sep 2020 10:49:05 +0300

> From: Fāng-ruì Sòng <maskray <at> google.com>
> Date: Sat, 12 Sep 2020 00:33:27 -0700
> Cc: 43195 <at> debbugs.gnu.org
> 
> > > I don't know whether it is worth mentioning that -fcommon/-fno-common does not
> > > cause a linker error when libtinfo.so is linked (a common/regular definition
> > > preempts a shared definition).
> > >
> > > -fno-common + libtinfo.a => multiple definition error
> >
> > OK, but the change works even in these other cases, right?
> 
> Yep! The new code should work in other cases, -fcommon/-fno-common x
> libtinfo.a/libtinfo.so

OK, so I'm closing the bug report.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.