unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#43195: [PATCH] Remove definitions of UP, BC and PC which should be provided by terminfo
@ 2020-09-04  0:57 Fangrui Song via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-09-04  7:36 ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Fangrui Song via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-09-04  0:57 UTC (permalink / raw)
  To: 43195; +Cc: Fangrui Song

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






^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#43195: [PATCH] Remove definitions of UP, BC and PC which should be provided by terminfo
  2020-09-04  0:57 bug#43195: [PATCH] Remove definitions of UP, BC and PC which should be provided by terminfo Fangrui Song via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-09-04  7:36 ` Eli Zaretskii
  2020-09-04 15:38   ` Fangrui Song via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2020-09-04  7:36 UTC (permalink / raw)
  To: Fangrui Song; +Cc: 43195, maskray

> Cc: Fangrui Song <maskray@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@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






^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#43195: [PATCH] Remove definitions of UP, BC and PC which should be provided by terminfo
  2020-09-04  7:36 ` Eli Zaretskii
@ 2020-09-04 15:38   ` Fangrui Song via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2020-09-12  7:19     ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Fangrui Song via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-09-04 15:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 43195

On 2020-09-04, Eli Zaretskii wrote:
>> Cc: Fangrui Song <maskray@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@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





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#43195: [PATCH] Remove definitions of UP, BC and PC which should be provided by terminfo
  2020-09-04 15:38   ` Fangrui Song via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-09-12  7:19     ` Eli Zaretskii
  2020-09-12  7:33       ` Unknown
  0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2020-09-12  7:19 UTC (permalink / raw)
  To: Fangrui Song; +Cc: 43195

> Date: Fri, 4 Sep 2020 08:38:03 -0700
> From: Fangrui Song <maskray@google.com>
> Cc: 43195@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?





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#43195: [PATCH] Remove definitions of UP, BC and PC which should be provided by terminfo
  2020-09-12  7:19     ` Eli Zaretskii
@ 2020-09-12  7:33       ` Unknown
  2020-09-12  7:49         ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Unknown @ 2020-09-12  7:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 43195

On Sat, Sep 12, 2020 at 12:19 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > Date: Fri, 4 Sep 2020 08:38:03 -0700
> > From: Fangrui Song <maskray@google.com>
> > Cc: 43195@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





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#43195: [PATCH] Remove definitions of UP, BC and PC which should be provided by terminfo
  2020-09-12  7:33       ` Unknown
@ 2020-09-12  7:49         ` Eli Zaretskii
  0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2020-09-12  7:49 UTC (permalink / raw)
  To: Fāng-ruì Sòng; +Cc: 43195-done

> From: Fāng-ruì Sòng <maskray@google.com>
> Date: Sat, 12 Sep 2020 00:33:27 -0700
> Cc: 43195@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.





^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-12  7:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04  0:57 bug#43195: [PATCH] Remove definitions of UP, BC and PC which should be provided by terminfo Fangrui Song via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-09-04  7:36 ` Eli Zaretskii
2020-09-04 15:38   ` Fangrui Song via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-09-12  7:19     ` Eli Zaretskii
2020-09-12  7:33       ` Unknown
2020-09-12  7:49         ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).