Patchwork Readline: Cleanup some warnings

login
register
mail settings
Submitter Alan Hayward
Date Jan. 30, 2019, 8:57 a.m.
Message ID <20190130085716.75179-1-alan.hayward@arm.com>
Download mbox | patch
Permalink /patch/31258/
State New
Headers show

Comments

Alan Hayward - Jan. 30, 2019, 8:57 a.m.
(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.
---
 readline/config.h.in | 21 +++++++++++++++++++++
 readline/histfile.c  |  3 ++-
 readline/util.c      |  6 +++---
 3 files changed, 26 insertions(+), 4 deletions(-)
Joel Brobecker - Jan. 31, 2019, 7:59 a.m.
> (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 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?

> ---
>  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)
Alan Hayward - Jan. 31, 2019, 10:02 a.m.
> 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

Patch

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;
 }