Re: [Axiom-developer] better pty patch

[Gabriel Dos Reis]

Waldek Hebisch <address@hidden> writes:

| Many systems (Linux, BSD and supposedly also Mac OS X) provide 'openpty'
| function as a preferred method to open a pty (for example on Linux
| 'openpty' will use Unix 98 ptys, but if those are not available it
| will fall back to legacy ptys).  'openpty' seem to be available
| in 'libutil', so in order to use it we must link to 'libutil'.
| 
| The patch below tries to 'openpty': add configure check and auguments
| compiler and linker argument.  There are some things which probably
| should be done in different way (please comment):
| 
| 1) configure defines 'HAVE_OPENPTY' preprocessor symbol, this symbol
|    is then propagated via 'DEFS' make variable and compile command
|    line.  Alternatively we could have a common header file generated
|    by configure

Yes, configure can generate a headef file for us to included, instead
of the current hackery of XXXplatform.   
Basically,

  (1) You need to issue

        AC_CONFIG_HEADERS([config/axiom-c-macros.h])

      in configure.ac.pamphlet.  This tells Autoconf (and especially
      Autoheader) that we are going to define CPP macros to
      communicate with thwe C programs, and we want to collect all
      those macros in the file config/axiom-c-macros.h.  Every
      AC_DEFINE you'll issue will end up there.

  (2) Run autoheader.  It should create a file named axiom-c-macros.h.in
      in the config/ directory.  Add it to the repository.

  (3) Modify the INC variable (this really should be set system-wide,
      I think), to include the path to config, e.g. add

        -I$(abs_top_builddir)/config

      [ The reason why I use $(abs_top_builddir) instead of the
       "simpler" $(top_builddir) is that there is a bug in Autoconf
       2.59 that would leave the variable top_builddir undefined.
       That is fixed in later versions, but unless we require Autoconf
       2.60 I think we're better with the absolute path. ]

  (4) Have the C programs include "axiom-c-macros.h"
   

[...]

| +\section{Extra libraries}
| +
| +Axiom supporting programs [[sman]] and [[clef]] use pseudo terminals.
| +To open pseudo terminals we use [[openpty]] if available, otherwise
| +we fall back to platform specific code.
| +
| +<<extra libraries>>=
| +AC_CHECK_FUNCS(openpty,, AC_CHECK_LIB(util,openpty, [AC_DEFINE(HAVE_OPENPTY)]
| + [EXTRA_LIBS="-lutil"]))
| +AC_SUBST(EXTRA_LIBS)
| +@

It is not enough to test for the library.  You have to test for the
headers <util.h> or <pty.h> too.  Furthermore, you need to check for the
declaration of the function openpty() -- not just its presence as
symbol in a library.

    # Check for <util.h>; if inexistent, check for <pty.h>
    AC_CHECK_HEADER([util.h], 
                    [AC_DEFINE([HAVE_UTIL_H])],
                    [AC_CHECK_HEADER([pty.h])
                    ]) # HAVE_UTIL_H or HAVE_PTY_H

    AC_CHECK_DECL([openpty], [], [],
                  [#if HAVE_UTIL_H
                   # include <util.h>
                   #elif HAVE_PTY_H
                   # include <pty.h>
                   #endif
                  ]) # HAVE_OPENPTY_DECL

    AC_CHECK_LIB([util], [openpty],
                 [AC_DEFINE([HAVE_OPENPTY])
                  EXTRA_LIBS="$EXTRA_LIBS -lutil"
                 ]) # HAVE_OPENPTY


[...]

| +We should really use autotools to check for Unix 98 pty support.
| +Before this is done below we hardcode information about each platform.
| +
|  \section{License}
|  <<license>>=
|  /*
| @@ -70,6 +62,9 @@
|  #include <stropts.h>
|  #endif
|  
| +#ifdef HAVE_OPENPTY
| +#include <pty.h>
| +#endif

This should be something like

   #ifdef HAVE_OPENPTY_DECL
   #  if HAVE_UTIL_H
   #    include <util.h>
   #  elif HAVE_PTY_H
   #    include <pty.h>
   #elif HAVE_OPENPTY
   /* The symbol openpty is allegedly present but its
      declaration was not found in any of the above headers.
      So, we fake it here.  Note this is NOT a prototype
      declaration.  */
   int openpty();
   #else 
   #  define AXIOM_DONT_USE_OPENPTY 1
   #endif

   #define AXIOM_USE_OPENPTY !AXIOM_DONT_USE_OPENPTY


|  
|  #include "openpty.H1"
|  
| @@ -104,7 +99,10 @@
|  #endif
|  
|  {
| -#if defined(SUNplatform) || defined (HP9platform) || defined(RTplatform) 
||defined(AIX370platform) || defined(BSDplatform)
| +#ifdef HAVE_OPENPTY

This should be

   #if AXIOM_USE_OPENPTY

-- Gaby