new file tparam.h to fix Emacs API glitch

new file tparam.h to fix Emacs API glitch

[Paul Eggert]

term.c declares and uses a function tparam, but its definition in
tparam.c uses an incompatible API.  This violates the
C standard, and possibly can cause runtime problems.
I plan to fix the problem in the trunk with the following patch,
which adds a new file tparam.h that contains the  API
so that any future incompatibilities are caught.

I thought I'd give a heads-up as this may require
a change to the dependencies in the Windows build process.

* tparam.h: New file.
* term.c, tparam.h: Include it.
* deps.mk (term.o, tparam.o): Depend on tparam.h.
* term.c (tputs, tgetent, tgetflag, tgetnum, tparam, tgetstr):
Move these decls to tparam.h, and make them agree with what
is actually in tparam.c.  The previous trick of using incompatible
decls in different modules does not conform to the C standard.
All callers of tparam changed to use tparam's actual API.
* tparam.c (tparam1, tparam, tgoto):
Use const pointers where appropriate.
=== modified file 'src/deps.mk'
--- src/deps.mk	2011-02-20 10:53:22 +0000
+++ src/deps.mk	2011-03-08 18:04:33 +0000
@@ -190,13 +190,13 @@
  term.o: term.c termchar.h termhooks.h termopts.h lisp.h globals.h $(config_h) \
     cm.h frame.h disptab.h keyboard.h character.h charset.h coding.h ccl.h \
     xterm.h msdos.h window.h keymap.h blockinput.h atimer.h systime.h \
-   systty.h syssignal.h $(INTERVALS_H) buffer.h ../lib/unistd.h
+   systty.h syssignal.h tparam.h $(INTERVALS_H) buffer.h ../lib/unistd.h
  termcap.o: termcap.c lisp.h $(config_h)
  terminal.o: terminal.c frame.h termchar.h termhooks.h charset.h coding.h \
     keyboard.h lisp.h globals.h $(config_h) dispextern.h composite.h systime.h \
     msdos.h
  terminfo.o: terminfo.c lisp.h globals.h $(config_h)
-tparam.o: tparam.c lisp.h $(config_h)
+tparam.o: tparam.c tparam.h lisp.h $(config_h)
  undo.o: undo.c buffer.h commands.h window.h dispextern.h msdos.h \
     lisp.h globals.h $(config_h)
  unexaix.o: unexaix.c lisp.h $(config_h)

=== modified file 'src/term.c'
--- src/term.c	2011-03-08 17:31:51 +0000
+++ src/term.c	2011-03-08 18:18:03 +0000
@@ -32,6 +32,7 @@
  #include "lisp.h"
  #include "termchar.h"
  #include "termopts.h"
+#include "tparam.h"
  #include "buffer.h"
  #include "character.h"
  #include "charset.h"
@@ -53,18 +54,6 @@
  static int been_here = -1;
  #endif

-/* For now, don't try to include termcap.h.  On some systems,
-   configure finds a non-standard termcap.h that the main build
-   won't find.  */
-extern void tputs (const char *, int, int (*)(int));
-extern int tgetent (char *, const char *);
-extern int tgetflag (char *id);
-extern int tgetnum (char *id);
-
-char *tparam (char *, char *, int, int, ...);
-
-extern char *tgetstr (char *, char **);
-
  #include "cm.h"
  #ifdef HAVE_X_WINDOWS
  #include "xterm.h"
@@ -262,7 +251,7 @@
    struct tty_display_info *tty = FRAME_TTY (f);

    if (tty->TS_set_scroll_region)
-    buf = tparam (tty->TS_set_scroll_region, 0, 0, start, stop - 1);
+    buf = tparam (tty->TS_set_scroll_region, 0, 0, start, stop - 1, 0, 0);
    else if (tty->TS_set_scroll_region_1)
      buf = tparam (tty->TS_set_scroll_region_1, 0, 0,
  		  FRAME_LINES (f), start,
@@ -859,7 +848,7 @@

    if (tty->TS_ins_multi_chars)
      {
-      buf = tparam (tty->TS_ins_multi_chars, 0, 0, len);
+      buf = tparam (tty->TS_ins_multi_chars, 0, 0, len, 0, 0, 0);
        OUTPUT1 (tty, buf);
        xfree (buf);
        if (start)
@@ -955,7 +944,7 @@

    if (tty->TS_del_multi_chars)
      {
-      buf = tparam (tty->TS_del_multi_chars, 0, 0, n);
+      buf = tparam (tty->TS_del_multi_chars, 0, 0, n, 0, 0, 0);
        OUTPUT1 (tty, buf);
        xfree (buf);
      }
@@ -997,7 +986,7 @@
      {
        raw_cursor_to (f, vpos, 0);
        tty_background_highlight (tty);
-      buf = tparam (multi, 0, 0, i);
+      buf = tparam (multi, 0, 0, i, 0, 0, 0);
        OUTPUT (tty, buf);
        xfree (buf);
      }
@@ -2125,7 +2114,7 @@
        ts = tty->standout_mode ? tty->TS_set_background : tty->TS_set_foreground;
        if (fg >= 0 && ts)
  	{
-          p = tparam (ts, NULL, 0, (int) fg);
+          p = tparam (ts, NULL, 0, (int) fg, 0, 0, 0);
  	  OUTPUT (tty, p);
  	  xfree (p);
  	}
@@ -2133,7 +2122,7 @@
        ts = tty->standout_mode ? tty->TS_set_foreground : tty->TS_set_background;
        if (bg >= 0 && ts)
  	{
-          p = tparam (ts, NULL, 0, (int) bg);
+          p = tparam (ts, NULL, 0, (int) bg, 0, 0, 0);
  	  OUTPUT (tty, p);
  	  xfree (p);
  	}

=== modified file 'src/tparam.c'
--- src/tparam.c	2011-01-15 23:16:57 +0000
+++ src/tparam.c	2011-03-08 18:16:15 +0000
@@ -21,6 +21,7 @@
  #include <config.h>
  #include <setjmp.h>
  #include "lisp.h"		/* for xmalloc */
+#include "tparam.h"

  #ifndef NULL
  #define NULL (char *) 0
@@ -38,11 +39,12 @@

     The fourth and following args to tparam serve as the parameter values.  */

-static char *tparam1 (char *string, char *outstring, int len, char *up, char *left, register int *argp);
+static char *tparam1 (char const *string, char *outstring, int len,
+		      char *up, char *left, int *argp);

-/* VARARGS 2 */
  char *
-tparam (char *string, char *outstring, int len, int arg0, int arg1, int arg2, int arg3)
+tparam (const char *string, char *outstring, int len,
+	int arg0, int arg1, int arg2, int arg3)
  {
    int arg[4];

@@ -59,7 +61,7 @@
  static char tgoto_buf[50];

  char *
-tgoto (char *cm, int hpos, int vpos)
+tgoto (const char *cm, int hpos, int vpos)
  {
    int args[2];
    if (!cm)
@@ -70,10 +72,11 @@
  }

  static char *
-tparam1 (char *string, char *outstring, int len, char *up, char *left, register int *argp)
+tparam1 (const char *string, char *outstring, int len,
+	 char *up, char *left, register int *argp)
  {
    register int c;
-  register char *p = string;
+  register const char *p = string;
    register char *op = outstring;
    char *outend;
    int outlen = 0;
@@ -277,4 +280,3 @@
  }

  #endif /* DEBUG */
-

=== added file 'src/tparam.h'
--- src/tparam.h	1970-01-01 00:00:00 +0000
+++ src/tparam.h	2011-03-08 18:16:45 +0000
@@ -0,0 +1,31 @@
+/* Interface definitions for termcap entries.
+
+Copyright (C) 2011  Free Software Foundation, Inc.
+
+This file is part of GNU Emacs.
+
+GNU Emacs is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+GNU Emacs is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
+
+
+/* Don't try to include termcap.h.  On some systems, configure finds a
+   non-standard termcap.h that the main build won't find.  */
+
+void tputs (const char *, int, int (*) (int));
+int tgetent (char *, const char *);
+int tgetflag (char *id);
+int tgetnum (char *id);
+char *tgetstr (char *, char **);
+char *tgoto (const char *, int, int);
+
+char *tparam (const char *, char *, int, int, int, int, int);

Re: new file tparam.h to fix Emacs API glitch

[Paul Eggert]

Following up on my previous message, termcap.c implements
part of the interface described in the new tparam.h, so it
should include it.  From the Windows point of view the only
part of the proposed patch below that should matter is the new
dependency of termcap.o on tparam.h.  The rest of the
patch inserts "const" to stay compatible with the interface.

* deps.mk (termcap.o): Depend on tparam.h.
* termcap.c: Include it.
(term_entry, tgetst1, find_capability, tgetnum, tgetflag, tgetstr):
(tgetst1, scan_file, compare_contin, name_match, tgetent):
Adjust to API's use of const char * rather than char *.
=== modified file 'src/deps.mk'
--- src/deps.mk	2011-03-08 18:26:34 +0000
+++ src/deps.mk	2011-03-08 18:41:16 +0000
@@ -191,7 +191,7 @@
     cm.h frame.h disptab.h keyboard.h character.h charset.h coding.h ccl.h \
     xterm.h msdos.h window.h keymap.h blockinput.h atimer.h systime.h \
     systty.h syssignal.h tparam.h $(INTERVALS_H) buffer.h ../lib/unistd.h
-termcap.o: termcap.c lisp.h $(config_h)
+termcap.o: termcap.c tparam.h lisp.h $(config_h)
  terminal.o: terminal.c frame.h termchar.h termhooks.h charset.h coding.h \
     keyboard.h lisp.h globals.h $(config_h) dispextern.h composite.h systime.h \
     msdos.h

=== modified file 'src/termcap.c'
--- src/termcap.c	2011-02-19 19:41:00 +0000
+++ src/termcap.c	2011-03-08 18:55:36 +0000
@@ -25,6 +25,7 @@
  #include <unistd.h>

  #include "lisp.h"
+#include "tparam.h"

  #ifndef NULL
  #define NULL (char *) 0
@@ -56,16 +57,16 @@

  /* The pointer to the data made by tgetent is left here
     for tgetnum, tgetflag and tgetstr to find.  */
-static char *term_entry;
+static const char *term_entry;

-static char *tgetst1 (char *ptr, char **area);
+static char *tgetst1 (const char *ptr, char **area);

  /* Search entry BP for capability CAP.
     Return a pointer to the capability (in BP) if found,
     0 if not found.  */

-static char *
-find_capability (register char *bp, register char *cap)
+static const char *
+find_capability (register const char *bp, register const char *cap)
  {
    for (; *bp; bp++)
      if (bp[0] == ':'
@@ -78,7 +79,7 @@
  int
  tgetnum (char *cap)
  {
-  register char *ptr = find_capability (term_entry, cap);
+  register const char *ptr = find_capability (term_entry, cap);
    if (!ptr || ptr[-1] != '#')
      return -1;
    return atoi (ptr);
@@ -87,7 +88,7 @@
  int
  tgetflag (char *cap)
  {
-  register char *ptr = find_capability (term_entry, cap);
+  register const char *ptr = find_capability (term_entry, cap);
    return ptr && ptr[-1] == ':';
  }

@@ -99,7 +100,7 @@
  char *
  tgetstr (char *cap, char **area)
  {
-  register char *ptr = find_capability (term_entry, cap);
+  register const char *ptr = find_capability (term_entry, cap);
    if (!ptr || (ptr[-1] != '=' && ptr[-1] != '~'))
      return NULL;
    return tgetst1 (ptr, area);
@@ -135,9 +136,10 @@
     or NULL if PTR is NULL.  */

  static char *
-tgetst1 (char *ptr, char **area)
+tgetst1 (const char *ptr, char **area)
  {
-  register char *p, *r;
+  register const char *p;
+  register char *r;
    register int c;
    register int size;
    char *ret;
@@ -218,7 +220,7 @@
    {
      register int last_p_param = 0;
      int remove_p_params = 1;
-    struct { char *beg; int len; } cut[11];
+    struct { const char *beg; int len; } cut[11];

      for (cut[0].beg = p = ret; p < r - 3; p++)
        {
@@ -263,7 +265,7 @@
  char PC;

  void
-tputs (register char *str, int nlines, register int (*outfun) (/* ??? */))
+tputs (register const char *str, int nlines, register int (*outfun) (int))
  {
    register int padcount = 0;
    register int speed;
@@ -327,10 +329,10 @@

  /* Forward declarations of static functions.  */

-static int scan_file (char *str, int fd, register struct termcap_buffer *bufp);
+static int scan_file (const char *str, int fd, struct termcap_buffer *bufp);
  static char *gobble_line (int fd, register struct termcap_buffer *bufp, char *append_end);
-static int compare_contin (register char *str1, register char *str2);
-static int name_match (char *line, char *name);
+static int compare_contin (const char *str1, const char *str2);
+static int name_match (const char *line, const char *name);

  #ifdef MSDOS /* MW, May 1993 */
  static int
@@ -355,17 +357,17 @@
     in it, and some other value otherwise.  */

  int
-tgetent (char *bp, char *name)
+tgetent (char *bp, const char *name)
  {
-  register char *termcap_name;
+  register const char *termcap_name;
    register int fd;
    struct termcap_buffer buf;
    register char *bp1;
-  char *tc_search_point;
-  char *term;
+  const char *tc_search_point;
+  const char *term;
    int malloc_size = 0;
    register int c;
-  char *tcenv = NULL;		/* TERMCAP value, if it contains :tc=.  */
+  const char *tcenv = NULL;	/* TERMCAP value, if it contains :tc=.  */
    char *indirect = NULL;	/* Terminal type in :tc= in TERMCAP value.  */
    int filep;

@@ -414,9 +416,11 @@
        if (!indirect)
  	{
  	  if (!bp)
-	    bp = termcap_name;
-	  else
-	    strcpy (bp, termcap_name);
+	    {
+	      term_entry = termcap_name;
+	      return 1;
+	    }
+	  strcpy (bp, termcap_name);
  	  goto ret;
  	}
        else
@@ -471,16 +475,15 @@
  	}

        /* Free old `term' if appropriate.  */
-      if (term != name)
-	free (term);
+      free (indirect);

        /* If BP is malloc'd by us, make sure it is big enough.  */
        if (malloc_size)
  	{
  	  int offset1 = bp1 - bp, offset2 = tc_search_point - bp;
  	  malloc_size = offset1 + buf.size;
-	  bp = termcap_name = (char *) xrealloc (bp, malloc_size);
-	  bp1 = termcap_name + offset1;
+	  termcap_name = bp = (char *) xrealloc (bp, malloc_size);
+	  bp1 = bp + offset1;
  	  tc_search_point = termcap_name + offset2;
  	}

@@ -498,7 +501,7 @@
        /* Does this entry refer to another terminal type's entry?
  	 If something is found, copy it into heap and null-terminate it.  */
        tc_search_point = find_capability (tc_search_point, "tc");
-      term = tgetst1 (tc_search_point, (char **) 0);
+      term = indirect = tgetst1 (tc_search_point, (char **) 0);
      }

    close (fd);
@@ -519,7 +522,7 @@
     or 0 if no entry is found in the file.  */

  static int
-scan_file (char *str, int fd, register struct termcap_buffer *bufp)
+scan_file (const char *str, int fd, register struct termcap_buffer *bufp)
  {
    register char *end;

@@ -556,9 +559,9 @@
     by termcap entry LINE.  */

  static int
-name_match (char *line, char *name)
+name_match (const char *line, const char *name)
  {
-  register char *tem;
+  register const char *tem;

    if (!compare_contin (line, name))
      return 1;
@@ -571,7 +574,7 @@
  }

  static int
-compare_contin (register char *str1, register char *str2)
+compare_contin (register const char *str1, register const char *str2)
  {
    register int c1, c2;
    while (1)
@@ -711,4 +714,3 @@
  }

  #endif /* TEST */
-

Re: new file tparam.h to fix Emacs API glitch

[Eli Zaretskii]

> Date: Tue, 08 Mar 2011 11:07:24 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> Following up on my previous message, termcap.c implements
> part of the interface described in the new tparam.h, so it
> should include it.

termcap.c is compiled only by the MS-DOS build.  No other build,
including the MS-Windows build and all Posix platforms, uses it.  So I
don't think you need to make any changes in it.  Let's just leave it
alone.

Re: new file tparam.h to fix Emacs API glitch

[Eli Zaretskii]

> Date: Tue, 08 Mar 2011 21:31:08 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > Date: Tue, 08 Mar 2011 11:07:24 -0800
> > From: Paul Eggert <eggert@cs.ucla.edu>
> > 
> > Following up on my previous message, termcap.c implements
> > part of the interface described in the new tparam.h, so it
> > should include it.
> 
> termcap.c is compiled only by the MS-DOS build.  No other build,
> including the MS-Windows build and all Posix platforms, uses it.  So I
> don't think you need to make any changes in it.  Let's just leave it
> alone.

I eventually did include tparam.h in termcap.c and modified termcap.c
to match the prototypes.  However, I made only the minimal changes
necessary for the implementation to match the declarations.  Since
termcap.c is used only by the MSDOS build, I don't see a reason to
maintain it beyond the absolute minimum.

Thanks.

Make tparam.h and terminfo.c consistent.

[Paul Eggert]

On 03/08/2011 10:32 AM, Paul Eggert wrote:
> term.c declares and uses a function tparam, but its definition in
> tparam.c uses an incompatible API.

Following up on that previous email of mine, I found another
incompatible-API declaration and use for tparam, this time in terminfo.c.
I plan to install the following patch to work around this problem.

I'm giving a heads-up now as this probably requires
a change to the dependencies in the MS-DOS and MS-Windows
build processes.

Make tparam.h and terminfo.c consistent.
* cm.c (tputs, tgoto, BC, UP): Remove extern decls.  Include
tparam.h instead, since it declares them.
* cm.h (PC): Remove extern decl; tparam.h now does this.
* deps.mk (cm.o, terminfo.o): Depend on tparam.h.
* terminfo.c: Include tparam.h, to check interfaces.
(tparm): Make 1st arg a const pointer in decl.  Put it at top level.
(tparam): Adjust signature to match interface in tparam.h;
this removes some undefined behavior.  Check that outstring and len
are zero, which they always are with Emacs.
* tparam.h (PC, BC, UP): New extern decls.
=== modified file 'src/cm.c'
--- src/cm.c	2011-03-09 02:12:00 +0000
+++ src/cm.c	2011-03-19 06:24:24 +0000
@@ -27,19 +27,11 @@
 #include "cm.h"
 #include "termhooks.h"
 #include "termchar.h"
-
-
-/* For now, don't try to include termcap.h.  On some systems,
-   configure finds a non-standard termcap.h that the main build
-   won't find.  */
-extern void tputs (const char *, int, int (*)(int));
-extern char *tgoto (const char *, int, int);
+#include "tparam.h"

 #define	BIG	9999		/* 9999 good on VAXen.  For 16 bit machines
 				   use about 2000.... */

-extern char *BC, *UP;
-
 int cost;		/* sums up costs */

 /* ARGSUSED */

=== modified file 'src/cm.h'
--- src/cm.h	2011-03-09 02:12:00 +0000
+++ src/cm.h	2011-03-19 06:28:37 +0000
@@ -96,8 +96,6 @@
     int cc_vabs;
   };

-extern char PC;			/* Pad character */
-
 /* Shorthand */
 #ifndef NoCMShortHand
 #define curY(tty)		(tty)->Wcm->cm_curY

=== modified file 'src/deps.mk'
--- src/deps.mk	2011-03-17 00:22:09 +0000
+++ src/deps.mk	2011-03-19 22:02:47 +0000
@@ -65,7 +65,7 @@
    window.h dispextern.h msdos.h frame.h termhooks.h \
    lisp.h globals.h $(config_h)
 cm.o: cm.c frame.h cm.h termhooks.h termchar.h dispextern.h msdos.h \
-   lisp.h globals.h $(config_h)
+   tparam.h lisp.h globals.h $(config_h)
 cmds.o: cmds.c syntax.h buffer.h character.h commands.h window.h lisp.h \
    globals.h $(config_h) msdos.h dispextern.h keyboard.h keymap.h systime.h \
    coding.h frame.h composite.h
@@ -196,7 +196,7 @@
 terminal.o: terminal.c frame.h termchar.h termhooks.h charset.h coding.h \
    keyboard.h lisp.h globals.h $(config_h) dispextern.h composite.h systime.h \
    msdos.h
-terminfo.o: terminfo.c lisp.h globals.h $(config_h)
+terminfo.o: terminfo.c tparam.h lisp.h globals.h $(config_h)
 tparam.o: tparam.c tparam.h lisp.h $(config_h)
 undo.o: undo.c buffer.h commands.h window.h dispextern.h msdos.h \
    lisp.h globals.h $(config_h)

=== modified file 'src/terminfo.c'
--- src/terminfo.c	2011-01-25 04:08:28 +0000
+++ src/terminfo.c	2011-03-19 22:32:45 +0000
@@ -17,6 +17,8 @@
 along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */

 #include <config.h>
+#include "tparam.h"
+
 #include <setjmp.h>
 #include "lisp.h"

@@ -33,18 +35,19 @@
    format is different too.
 */

+extern char *tparm (const char *str, ...);
+
+
 char *
-tparam (char *string, char *outstring,
-	int len, int arg1, int arg2, int arg3, int arg4,
-	int arg5, int arg6, int arg7, int arg8, int arg9)
+tparam (const char *string, char *outstring, int len,
+	int arg1, int arg2, int arg3, int arg4)
 {
   char *temp;
-  extern char *tparm (char *str, ...);
-
-  temp = tparm (string, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9);
-  if (outstring == 0)
-    outstring = ((char *) (xmalloc ((strlen (temp)) + 1)));
-  strcpy (outstring, temp);
-  return outstring;
+
+  /* Emacs always should pass a null OUTSTRING and zero LEN.  */
+  if (outstring || len)
+    abort ();
+
+  temp = tparm (string, arg1, arg2, arg3, arg4);
+  return xstrdup (temp);
 }
-

=== modified file 'src/tparam.h'
--- src/tparam.h	2011-03-09 05:46:35 +0000
+++ src/tparam.h	2011-03-19 06:32:29 +0000
@@ -29,3 +29,7 @@
 char *tgoto (const char *, int, int);

 char *tparam (const char *, char *, int, int, int, int, int);
+
+extern char PC;
+extern char *BC;
+extern char *UP;

Re: Make tparam.h and terminfo.c consistent.

[Eli Zaretskii]

> Date: Sat, 19 Mar 2011 15:49:25 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> On 03/08/2011 10:32 AM, Paul Eggert wrote:
> > term.c declares and uses a function tparam, but its definition in
> > tparam.c uses an incompatible API.
> 
> Following up on that previous email of mine, I found another
> incompatible-API declaration and use for tparam, this time in terminfo.c.
> I plan to install the following patch to work around this problem.
> 
> I'm giving a heads-up now as this probably requires
> a change to the dependencies in the MS-DOS and MS-Windows
> build processes.

Only MS-DOS, actually.  terminfo.c is not used by these two ports, and
cm.c is used only by MS-DOS.