Re: [PATCH] readline/util.c: use '%d' instead of 'ld' to avoid compiling warning

Re: [PATCH] readline/util.c: use '%d' instead of 'ld' to avoid compiling warning

[Chen Gang]

On 05/30/2014 04:27 PM, Pedro Alves wrote:
> On 05/30/2014 03:50 AM, Chen Gang wrote:
> 
>> If we have a tool to tell members (especially new members) the related
>> mailing addresses (e.g. "scripts/get_maintainer.pl" under Linux kernel
>> and/or Qemu), that will be cool!
> 
> I agree.  If you'd like to contribute one, it'll be gladly accepted.
> The information can currently be found in the MAINTAINERS files
> in the several directories in the tree.
> 

I am glad to. I am not quite familiar with perl, but it is not the
excuse to let myself stop -- I shall try.

At present, I am scanning binutils for all compiling warnings, and try
to fix them. I guess I need about 1-2 month (I am not quite familiar
with binutils, either have no enough free time for it).

So may I finish scanning warnings, then try "get_maintainers.pl"? (also
welcome any members to scan warnings and/or try "get_maintainers.pl")


[...]

Thanks.
-- 
Chen Gang

Open, share and attitude like air, water and life which God blessed

[PATCH] readline/util.c: use '%d' instead of 'ld' to avoid compiling warning

[Chen Gang]

getpid() will return 'pid_t' which is always 'int', so need use '%d'
instead of '%ld' in sprintf().

The related warning (cross-compile aarch64):

  ../../binutils-gdb/readline/util.c: In function ‘_rl_tropen’:
  ../../binutils-gdb/readline/util.c:510:3: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 3 has type ‘__pid_t’ [-Wformat]


Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 readline/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/readline/util.c b/readline/util.c
index 321dee2..05c8abc 100644
--- a/readline/util.c
+++ b/readline/util.c
@@ -507,7 +507,7 @@ _rl_tropen ()
 
   if (_rl_tracefp)
     fclose (_rl_tracefp);
-  sprintf (fnbuf, "/var/tmp/rltrace.%ld", getpid());
+  sprintf (fnbuf, "/var/tmp/rltrace.%d", getpid());
   unlink(fnbuf);
   _rl_tracefp = fopen (fnbuf, "w+");
   return _rl_tracefp != 0;
-- 
1.9.2.459.g68773ac

Re: [PATCH] readline/util.c: use '%d' instead of 'ld' to avoid compiling warning

[Alan Modra]

On Fri, May 30, 2014 at 08:11:41AM +0800, Chen Gang wrote:
> getpid() will return 'pid_t' which is always 'int', so need use '%d'
> instead of '%ld' in sprintf().

Please do not copy many lists and/or maintainers.  This patch should
have gone just to gdb-patches@sourceware.org, since readline belongs
to the gdb project.

Also, gdb and binutils generally require patches to be supplied with a
ChangeLog entry.  See other posts on the mailing list.

Now, on to the substance of your patch.  It's wrong to claim that
"pid_t" is always "int".  I know of systems that use or used to use
"short".  In fact
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html
says

"The implementation shall support one or more programming environments
in which the widths of blksize_t, pid_t, size_t, ssize_t, and
suseconds_t are no greater than the width of type long."

So the correct patch would be to cast the return of getpid() to long,
continuing to use '%ld' in the format string.

> The related warning (cross-compile aarch64):
> 
>   ../../binutils-gdb/readline/util.c: In function ‘_rl_tropen’:
>   ../../binutils-gdb/readline/util.c:510:3: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 3 has type ‘__pid_t’ [-Wformat]

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] readline/util.c: use '%d' instead of 'ld' to avoid compiling warning

[Alan Modra]

On Fri, May 30, 2014 at 11:45:07AM +0930, Alan Modra wrote:
> On Fri, May 30, 2014 at 08:11:41AM +0800, Chen Gang wrote:
> > getpid() will return 'pid_t' which is always 'int', so need use '%d'
> > instead of '%ld' in sprintf().
> 
> Please do not copy many lists and/or maintainers.  This patch should
> have gone just to gdb-patches@sourceware.org, since readline belongs
> to the gdb project.

Oops, maybe *I* should have read readline/README before sending my
email.  readline/README tells you the correct mailing list!

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] readline/util.c: use '%d' instead of 'ld' to avoid compiling warning

[Chen Gang]

On 05/30/2014 10:22 AM, Alan Modra wrote:
> On Fri, May 30, 2014 at 11:45:07AM +0930, Alan Modra wrote:
>> On Fri, May 30, 2014 at 08:11:41AM +0800, Chen Gang wrote:
>>> getpid() will return 'pid_t' which is always 'int', so need use '%d'
>>> instead of '%ld' in sprintf().
>>
>> Please do not copy many lists and/or maintainers.  This patch should
>> have gone just to gdb-patches@sourceware.org, since readline belongs
>> to the gdb project.
> 
> Oops, maybe *I* should have read readline/README before sending my
> email.  readline/README tells you the correct mailing list!
> 

OK, thanks.

And excuse me, originally, I did not know that readline only belongs to gdb.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] readline/util.c: use '%d' instead of 'ld' to avoid compiling warning

[Chen Gang]

On 05/30/2014 10:15 AM, Alan Modra wrote:
> On Fri, May 30, 2014 at 08:11:41AM +0800, Chen Gang wrote:
>> getpid() will return 'pid_t' which is always 'int', so need use '%d'
>> instead of '%ld' in sprintf().
> 
> Please do not copy many lists and/or maintainers.  This patch should
> have gone just to gdb-patches@sourceware.org, since readline belongs
> to the gdb project.
> 
> Also, gdb and binutils generally require patches to be supplied with a
> ChangeLog entry.  See other posts on the mailing list.
> 

OK, thanks.

If we have a tool to tell members (especially new members) the related
mailing addresses (e.g. "scripts/get_maintainer.pl" under Linux kernel
and/or Qemu), that will be cool!


> Now, on to the substance of your patch.  It's wrong to claim that
> "pid_t" is always "int".  I know of systems that use or used to use
> "short".  In fact
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html
> says
> 
> "The implementation shall support one or more programming environments
> in which the widths of blksize_t, pid_t, size_t, ssize_t, and
> suseconds_t are no greater than the width of type long."
> 
> So the correct patch would be to cast the return of getpid() to long,
> continuing to use '%ld' in the format string.
> 

That sounds fine to me.

Please help integrate it into mainline. If need me send patch v2, please
let me know (no reply means "will integrate the correct fix into
mainline by maintainers directly").


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] readline/util.c: use '%d' instead of 'ld' to avoid compiling warning

[Pedro Alves]

On 05/30/2014 03:50 AM, Chen Gang wrote:

> If we have a tool to tell members (especially new members) the related
> mailing addresses (e.g. "scripts/get_maintainer.pl" under Linux kernel
> and/or Qemu), that will be cool!

I agree.  If you'd like to contribute one, it'll be gladly accepted.
The information can currently be found in the MAINTAINERS files
in the several directories in the tree.

> Please help integrate it into mainline. If need me send patch v2, please
> let me know (no reply means "will integrate the correct fix into
> mainline by maintainers directly").

We prefer avoiding diverging our readline copy from upstream if
possible.  Please send a corrected patch to the readline list.
Once it is accepted there, we can then backport the fix into our tree.

Thanks!

-- 
Pedro Alves

Re: [PATCH] readline/util.c: use '%d' instead of 'ld' to avoid compiling warning

[Chen Gang]

On 05/30/2014 05:21 PM, Andreas Schwab wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> We prefer avoiding diverging our readline copy from upstream if
>> possible.  Please send a corrected patch to the readline list.
>> Once it is accepted there, we can then backport the fix into our tree.
> 
> It has already been fixed upstream more than two years ago, see
> <http://git.savannah.gnu.org/cgit/bash.git/diff/lib/readline/util.c?id=5a31873>.
> 
> Andreas.
> 

That is fine to me.

Thanks.
-- 
Chen Gang

Open, share and attitude like air, water and life which God blessed

Re: [PATCH] readline/util.c: use '%d' instead of 'ld' to avoid compiling warning

[Chen Gang]

On 5/31/14 0:43, Chen Gang wrote:
> On 05/30/2014 05:21 PM, Andreas Schwab wrote:
>> Pedro Alves <palves@redhat.com> writes:
>>
>>> We prefer avoiding diverging our readline copy from upstream if
>>> possible.  Please send a corrected patch to the readline list.
>>> Once it is accepted there, we can then backport the fix into our tree.
>>
>> It has already been fixed upstream more than two years ago, see
>> <http://git.savannah.gnu.org/cgit/bash.git/diff/lib/readline/util.c?id=5a31873>.
>>
>> Andreas.
>>
> 
> That is fine to me.
> 

It seems, it is still out of main line (master branch in 2014-09-15),
please help integrate it into main line when you have time.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed