[bug-inetutils] Re: ftp: command processing

[Simon Josefsson]

"Debarshi 'Rishi' Ray" <address@hidden> writes:

>> However, I just remembered that there is a readline module in gnulib
>> (which, incidentally, uses getline).  Wouldn't using that module
>> simplify your code a lot?  Then you can forget about the non-readline
>> situation, and let gnulib take care of that.
>
> As I noted already, we can use gnulib and forget about the
> non-readline situation, but that would require changes throughout the
> code in order to ensure uniformity. Should we make all that change in
> one single patch? Or do it in a separate one?

Hm, yes, some changes will need to be made at the same time, since
configure.ac checks for readline manually now, which won't be necessary
and may be potentially harmful if the gnulib readline module is used.

A rgrep on readline reveals that readline is not used in that many
places:

address@hidden:~/src/inetutils$ rgrep readline *
ChangeLog:      * ftp/main.c [HAVE_READLINE_READLINE_H]: Include 
<readline/readline.h>
ChangeLog:      (AC_CHECK_HEADERS): Add readline/readline.h
ChangeLog:      * ftp/main.c: Make sure line from readline isn't larger than
ChangeLog:      readline, since the latter might depend on the former.
ChangeLog:        readline check: assign -lreadline to the separate AC_SUBST
ChangeLog:      strcpy. Unfortunately, the readline() changes cause this to be
ChangeLog.0:    Added checks for libtermcap, libreadline, and libopie.
configure.ac:# We want to use libreadline if it is available.  However, it 
seems that the
configure.ac:# guile hackers have had problems because there is a Solaris 
libreadline
configure.ac:# that is different from the GNU libreadline; therefore we test 
for an obscure
configure.ac:# function in the GNU libreadline.
configure.ac:#AC_CHECK_LIB(readline, rl_bind_key,,, -ltermcap)
configure.ac:AC_CHECK_LIB(readline, rl_bind_key,
configure.ac:             [LIBREADLINE=-lreadline
configure.ac:              [Define to one if you have -lreadline])],
configure.ac:             readline/readline.h \
ftp/main.c:# include <readline/readline.h>
ftp/main.c:      line = readline (prompt);
NEWS:* If you have the GNU readline library installed, it will now be used
TODO:(readline support, &c), and add readline to the distribution.
uucpd/uucpd.c:  if (readline (user, sizeof user) < 0)
uucpd/uucpd.c:      if (readline (passwd, sizeof passwd) < 0)
uucpd/uucpd.c:readline (register char *p, register int n)
address@hidden:~/src/inetutils$ 

However, it seems the readline function in uucpd.c is not the real one.

So it seems ftp is the only user of the real readline function.  Thus I
think it would be quite simple to make the change, no?  Just remove all
readline stuff from configure.ac, remove the non-readline case in
ftp/main.c, and add the gnulib readline module, and you should be set.

I think it is up to the other inetutils hackers to decide.  I'm willing
to propose a patch for this, if you want to concentrate on argp'ifying.

/Simon