Re: [PATCH] Readline: Cleanup some warnings

[Alan Hayward <Alan.Hayward@arm.com>]

> On 31 Jan 2019, at 07:59, Joel Brobecker <brobecker@adacore.com> wrote:
> 
>> (Posted this first to binutils, then was directed back here and
>> pointed at the upstream readline.  Looking at the upstream
>> readline it has already fixed the issues below in the same way.
>> Tested by running the gdb testsuite - couldn't see a readline
>> specific suite.) 
>> 
>> Cleanup the readline warnings that gdb buildbot complains about.
>> 
>> To prevent wcwidth missing declaration warnings, add the SOURCE /
>> EXTENSION macros to config.in that have already checked for in
>> configure.  Use the exact same list as GDB - it seemed sensible
>> to add all of them.
>> 
>> Ensure pid is a long before printing as one.  Also fix GNU style.
>> 
>> Check the return value of write the same way as history_do_write ().
>> 
>> These changes are consistent with upstream readline.
>> 
>> readline/ChangeLog.gdb:
>> 
>> 2019-01-30  Alan Hayward  <alan.hayward@arm.com>
>> 
>> 	* config.h.in: Add SOURCE/EXTENSION macros.
>> 	* histfile.c (history_truncate_file): Check return of write.
>> 	* util.c: Ensure pid is long.
> 
> If it is a backport

Technically, not a back port because I wrote the changes then realised
they are the same as upstream.  Just a quick thought - would it help
with future rebasing if I ensured the changes were *exactly* the same?
(For example, the config.h.in changes are in a different place in the 
file with different comments).

> from mainline readline, and you've run
> the testsuite (readline is being necessarily extensively covered,
> but at least it is used implicitly, since it provides the framework
> for the interactive prompt, which is being driven via expect/tcl),
> it's OK to push.
> 
> Just one question: Any reason you didn't list the name of the function
> being modified, in the change to util.c?

Missed that!

> 
>> ---
>> readline/config.h.in | 21 +++++++++++++++++++++
>> readline/histfile.c  |  3 ++-
>> readline/util.c      |  6 +++---
>> 3 files changed, 26 insertions(+), 4 deletions(-)
>> 
>> diff --git a/readline/config.h.in b/readline/config.h.in
>> index 86d86cfa3d..81575f43f7 100644
>> --- a/readline/config.h.in
>> +++ b/readline/config.h.in
>> @@ -271,3 +271,24 @@
>> #    define USE_VARARGS
>> #  endif
>> #endif
>> +
>> +/* Enable extensions on AIX 3, Interix.  */
>> +#ifndef _ALL_SOURCE
>> +# undef _ALL_SOURCE
>> +#endif
>> +/* Enable GNU extensions on systems that have them.  */
>> +#ifndef _GNU_SOURCE
>> +# undef _GNU_SOURCE
>> +#endif
>> +/* Enable threading extensions on Solaris.  */
>> +#ifndef _POSIX_PTHREAD_SEMANTICS
>> +# undef _POSIX_PTHREAD_SEMANTICS
>> +#endif
>> +/* Enable extensions on HP NonStop.  */
>> +#ifndef _TANDEM_SOURCE
>> +# undef _TANDEM_SOURCE
>> +#endif
>> +/* Enable general extensions on Solaris.  */
>> +#ifndef __EXTENSIONS__
>> +# undef __EXTENSIONS__
>> +#endif
>> diff --git a/readline/histfile.c b/readline/histfile.c
>> index fffeb3fd31..56cbbf0498 100644
>> --- a/readline/histfile.c
>> +++ b/readline/histfile.c
>> @@ -407,7 +407,8 @@ history_truncate_file (fname, lines)
>>      truncate to. */
>>   if (bp > buffer && ((file = open (filename, O_WRONLY|O_TRUNC|O_BINARY, 0600)) != -1))
>>     {
>> -      write (file, bp, chars_read - (bp - buffer));
>> +      if (write (file, bp, chars_read - (bp - buffer)) < 0)
>> +	rv = errno;
>> 
>> #if defined (__BEOS__)
>>       /* BeOS ignores O_TRUNC. */
>> diff --git a/readline/util.c b/readline/util.c
>> index d402fce842..13bd00c09c 100644
>> --- a/readline/util.c
>> +++ b/readline/util.c
>> @@ -515,11 +515,11 @@ _rl_tropen ()
>> 	   (sh_get_env_value ("TEMP")
>> 	    ? sh_get_env_value ("TEMP")
>> 	    : "."),
>> -	   getpid());
>> +	   getpid ());
>> #else
>> -  sprintf (fnbuf, "/var/tmp/rltrace.%ld", getpid());
>> +  sprintf (fnbuf, "/var/tmp/rltrace.%ld", (long) getpid ());
>> #endif
>> -  unlink(fnbuf);
>> +  unlink (fnbuf);
>>   _rl_tracefp = fopen (fnbuf, "w+");
>>   return _rl_tracefp != 0;
>> }
>> -- 
>> 2.17.2 (Apple Git-113)
> 
> -- 
> Joel