Re: [PATCH] Readline: Cleanup some warnings

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

> On 31 Jan 2019, at 10:02, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
> 
> 
>> 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.

Thinking about what you said, I’ve updated the config.h.in code so it
is a direct backport, and pushed.  Functionally it’s the same as the
original version. Patch pasted below.



diff --git a/readline/config.h.in b/readline/config.h.in
index 86d86cfa3d..c194e761a4 100644
--- a/readline/config.h.in
+++ b/readline/config.h.in
@@ -1,5 +1,15 @@
 /* config.h.in.  Maintained by hand. */

+/* Template definitions for autoconf */
+#undef __EXTENSIONS__
+#undef _ALL_SOURCE
+#undef _GNU_SOURCE
+#undef _POSIX_SOURCE
+#undef _POSIX_1_SOURCE
+#undef _POSIX_PTHREAD_SEMANTICS
+#undef _TANDEM_SOURCE
+#undef _MINIX
+
 /* Define NO_MULTIBYTE_SUPPORT to not compile in support for multibyte
    characters, even if the OS supports them. */
 #undef NO_MULTIBYTE_SUPPORT
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;
 }