[ob] More warnings; Call for assistance

[ob] More warnings; Call for assistance

[Daniel Jacobowitz]

Here's the last two that are particularly straightforward.  Committed as
obvious.  But there's more:

/space/fsf/commit/src/gdb/printcmd.c:1964: warning: format not a string literal, argument types not checked
/space/fsf/commit/src/gdb/printcmd.c:1970: warning: format not a string literal, argument types not checked
/space/fsf/commit/src/gdb/printcmd.c:1977: warning: format not a string literal, argument types not checked
/space/fsf/commit/src/gdb/printcmd.c:1987: warning: format not a string literal, argument types not checked
/space/fsf/commit/src/gdb/expprint.c:180: warning: pointer targets in passing argument 2 of 'current_language->la_printstr' differ in signedness
/space/fsf/commit/src/gdb/expprint.c:194: warning: pointer targets in passing argument 2 of 'current_language->la_printstr' differ in signedness
/space/fsf/commit/src/gdb/expprint.c:273: warning: pointer targets in passing argument 2 of 'current_language->la_printstr' differ in signedness
/space/fsf/commit/src/gdb/kod.c:124: warning: pointer targets in passing argument 4 of 'target_read_partial' differ in signedness
/space/fsf/commit/src/gdb/coff-pe-read.c:299: warning: pointer targets in assignment differ in signedness
/space/fsf/commit/src/gdb/coff-pe-read.c:338: warning: pointer targets in passing argument 1 of 'add_pe_exported_sym' differ in signedness
/space/fsf/commit/src/gdb/dwarf2read.c:7059: warning: pointer targets in passing argument 1 of 'store_unsigned_integer' differ in signedness
/space/fsf/commit/src/gdb/dwarf2read.c:9235: warning: pointer targets in assignment differ in signedness
/space/fsf/commit/src/gdb/dwarf2read.c:9260: warning: pointer targets in assignment differ in signedness
/space/fsf/commit/src/gdb/corefile.c:332: warning: pointer targets in passing argument 2 of 'read_memory' differ in signedness
/space/fsf/commit/src/gdb/ada-lang.c:357: warning: pointer targets in passing argument 2 of 'target_read_memory' differ in signedness
/space/fsf/commit/src/gdb/ada-lang.c:1217: warning: pointer targets in passing argument 1 of 'modify_field' differ in signedness
/space/fsf/commit/src/gdb/ada-lang.c:2173: warning: pointer targets in passing argument 2 of 'read_memory' differ in signedness
/space/fsf/commit/src/gdb/ada-lang.c:2178: warning: pointer targets in passing argument 1 of 'move_bits' differ in signedness
/space/fsf/commit/src/gdb/ada-lang.c:2181: warning: pointer targets in passing argument 1 of 'move_bits' differ in signedness
/space/fsf/commit/src/gdb/ada-lang.c:2182: warning: pointer targets in passing argument 2 of 'write_memory' differ in signedness
/space/fsf/commit/src/gdb/ada-lang.c:3749: warning: pointer targets in passing argument 1 of 'modify_general_field' differ in signedness
/space/fsf/commit/src/gdb/ada-lang.c:3753: warning: pointer targets in passing argument 1 of 'modify_general_field' differ in signedness
/space/fsf/commit/src/gdb/ada-lang.c:3761: warning: pointer targets in passing argument 1 of 'modify_general_field' differ in signedness
/space/fsf/commit/src/gdb/ada-lang.c:3766: warning: pointer targets in passing argument 1 of 'modify_general_field' differ in signedness
/space/fsf/commit/src/gdb/dwarf2-frame.c:1735: warning: pointer targets in assignment differ in signedness
/space/fsf/commit/src/gdb/dwarf2-frame.c:1763: warning: pointer targets in assignment differ in signedness

The printcmd.c warnings we've been looking at for ages; they should be
fixed, but it doesn't have to be this week.  The other warnings are mostly
things that will require large overhauls to get right, for instance:

LA_PRINT_STRING takes a const gdb_byte * argument for the string.  But this
is a NUL-terminated string in the debugger's memory; I think stepping back
to char * is the best fix here.  That's the expprint.c warnings.

Two of the dwarf2read.c warnings are caused by the use of char * buffers for
debugger data.  This is binary data, using gdb_byte * seems reasonable, but
it's all over the file.

The other in that file is caused by SYMBOL_VALUE_BYTES which needs
gdb_bytizing.

I have no idea what the state of kod.c is nowadays.  Does it still work?
Does anyone still use it?  Is it still a good idea?  The warning comes from
a multi-file interface defined to use char*.

And so forth.  I don't think I'm going to fix any of the rest.

-- 
Daniel Jacobowitz
CodeSourcery

2006-01-17  Daniel Jacobowitz  <dan@codesourcery.com>

	* complaints.c (stop_whining): Make signed.
	* linux-thread-db.c (thread_db_store_registers): Use gdb_byte.

Index: complaints.c
===================================================================
RCS file: /cvs/src/src/gdb/complaints.c,v
retrieving revision 1.23
diff -u -p -r1.23 complaints.c
--- complaints.c	17 Dec 2005 22:33:59 -0000	1.23
+++ complaints.c	17 Jan 2006 14:56:57 -0000
@@ -1,7 +1,7 @@
 /* Support for complaint handling during symbol reading in GDB.
 
    Copyright (C) 1990, 1991, 1992, 1993, 1995, 1998, 1999, 2000, 2002,
-   2004, 2005 Free Software Foundation, Inc.
+   2004, 2005, 2006 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -161,7 +161,7 @@ find_complaint (struct complaints *compl
    before we stop whining about it?  Default is no whining at all,
    since so many systems have ill-constructed symbol files.  */
 
-static unsigned int stop_whining = 0;
+static int stop_whining = 0;
 
 /* Print a complaint, and link the complaint block into a chain for
    later handling.  */
Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.12
diff -u -p -r1.12 linux-thread-db.c
--- linux-thread-db.c	17 Dec 2005 22:34:01 -0000	1.12
+++ linux-thread-db.c	17 Jan 2006 14:56:57 -0000
@@ -1,6 +1,6 @@
 /* libthread_db assisted debugging support, generic parts.
 
-   Copyright (C) 1999, 2000, 2001, 2003, 2004, 2005
+   Copyright (C) 1999, 2000, 2001, 2003, 2004, 2005, 2006
    Free Software Foundation, Inc.
 
    This file is part of GDB.
@@ -1048,7 +1048,7 @@ thread_db_store_registers (int regno)
 
   if (regno != -1)
     {
-      char raw[MAX_REGISTER_SIZE];
+      gdb_byte raw[MAX_REGISTER_SIZE];
 
       deprecated_read_register_gen (regno, raw);
       thread_db_fetch_registers (-1);

Re: [ob] More warnings; Call for assistance

[Daniel Jacobowitz]

On Tue, Jan 17, 2006 at 10:17:31AM -0500, Daniel Jacobowitz wrote:
> @@ -161,7 +161,7 @@ find_complaint (struct complaints *compl
>     before we stop whining about it?  Default is no whining at all,
>     since so many systems have ill-constructed symbol files.  */
>  
> -static unsigned int stop_whining = 0;
> +static int stop_whining = 0;
>  
>  /* Print a complaint, and link the complaint block into a chain for
>     later handling.  */

Oops!  I did check that this was correct within the logic of the file,
but it still causes three test failures.  gdb.cp/maint.exp does
"set complaints -1" and expects to get all complaints.

This is a var_zinteger, which is supposed to be signed - thus my
change.  What maint.exp is doing is undocumented, and seems passingly
illogical to me; does anyone object to my changing it to "set
complaints 1000"?

The manual says:

`set complaints LIMIT'
     Permits GDB to output LIMIT complaints about each type of unusual
     symbols before becoming silent about the problem.  Set LIMIT to
     zero to suppress all complaints; set it to a large number to
     prevent complaints from being suppressed.


-- 
Daniel Jacobowitz
CodeSourcery

Re: [ob] More warnings; Call for assistance

[Jim Blandy]

Looking at the printcmd.c warnings, there's the following note in Makefile.in:

# FIXME: cagney/2003-08-10: Do not try to build "printcmd.c" with
# -Wformat-nonliteral.  It needs to be overhauled so that it doesn't
# pass user input strings as the format parameter to host printf
# function calls.
printcmd.o: $(srcdir)/printcmd.c
	$(CC) -c $(INTERNAL_WARN_CFLAGS) $(NO_WERROR_CFLAGS) $(srcdir)/printcmd.c

I gather what this is suggesting is that we have a big switch
selecting an appropriate call to printf that uses a fixed format
string.

There will be dozens of cases there, due to the modifiers (h, l, ll,
precision, leading sign, alternative form).  The precisions will need
to be parsed when present; sometimes they are minimum values,
sometimes they are maximum values.  Since we check the number and type
of the arguments, I think -Wformat-nonliteral is the right answer
here.  I don't see a benefit to making this change that justifies the
risk of mistakes.

What do folks think?

Re: [ob] More warnings; Call for assistance

[Daniel Jacobowitz]

On Tue, Jan 17, 2006 at 11:37:29AM -0800, Jim Blandy wrote:
> Looking at the printcmd.c warnings, there's the following note in Makefile.in:
> 
> # FIXME: cagney/2003-08-10: Do not try to build "printcmd.c" with
> # -Wformat-nonliteral.  It needs to be overhauled so that it doesn't
> # pass user input strings as the format parameter to host printf
> # function calls.
> printcmd.o: $(srcdir)/printcmd.c
> 	$(CC) -c $(INTERNAL_WARN_CFLAGS) $(NO_WERROR_CFLAGS) $(srcdir)/printcmd.c
> 
> I gather what this is suggesting is that we have a big switch
> selecting an appropriate call to printf that uses a fixed format
> string.
> 
> There will be dozens of cases there, due to the modifiers (h, l, ll,
> precision, leading sign, alternative form).  The precisions will need
> to be parsed when present; sometimes they are minimum values,
> sometimes they are maximum values.  Since we check the number and type
> of the arguments, I think -Wformat-nonliteral is the right answer
> here.  I don't see a benefit to making this change that justifies the
> risk of mistakes.
> 
> What do folks think?

I disagree, because (IIRC) Debian users have filed at least two bugs
where failures in this code have led to user input crashing GDB:

  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=186037
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=231162

The advantages of static format checking, in the face of that, are
pretty large.  If we can simplify this or else avoid the use of
standard printf, we won't have these kinds of bugs.

However, I'd strongly prefer to address the pointer problems first
before returning to worry around the edges of this one :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [ob] More warnings; Call for assistance

[Eli Zaretskii]

> Date: Tue, 17 Jan 2006 14:46:24 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> I disagree, because (IIRC) Debian users have filed at least two bugs
> where failures in this code have led to user input crashing GDB:
> 
>   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=186037
>   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=231162

These aren't crashes, GDB prints a legible error message and returns
to top level.  However, internal_error is not the best idea for these
situations, so we probably should arrange for GDB to mutter something
about possibly bad format instead, and not to ask whether to dump
core.

> The advantages of static format checking, in the face of that, are
> pretty large.

No matter what you do, as long as users are typing format strings, it
will always be possible for a format to exceed our wildest
imagination.  You cannot beat that, unless you are willing to
artificially restrict users to some safe and/or easily parsable subset
of valid formats, which I think we shouldn't do.

> avoid the use of standard printf

Avoid? how? are you saying that we should write our own version of
printf?

Re: [ob] More warnings; Call for assistance

[Eli Zaretskii]

> Date: Tue, 17 Jan 2006 11:37:29 -0800
> From: Jim Blandy <jimb@red-bean.com>
> 
> I gather what this is suggesting is that we have a big switch
> selecting an appropriate call to printf that uses a fixed format
> string.
> 
> There will be dozens of cases there, due to the modifiers (h, l, ll,
> precision, leading sign, alternative form).  The precisions will need
> to be parsed when present; sometimes they are minimum values,
> sometimes they are maximum values.  Since we check the number and type
> of the arguments, I think -Wformat-nonliteral is the right answer
> here.  I don't see a benefit to making this change that justifies the
> risk of mistakes.
> 
> What do folks think?

I think that GCC warning is silly to begin with, so
"-Wformat-nonliteral" sounds good to me.

Re: [ob] More warnings; Call for assistance

[Eli Zaretskii]

> Date: Tue, 17 Jan 2006 10:17:31 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> /space/fsf/commit/src/gdb/expprint.c:180: warning: pointer targets in passing argument 2 of 'current_language->la_printstr' differ in signedness

Is it really worth your trouble fixing this?  AFAIK, the next release
of GCC is supposed to turn this warning off by default, so why fix
something that ain't broken in the first place, if it's gonna bug only
those users who have GCC 4.0.x?

Re: [ob] More warnings; Call for assistance

[Daniel Jacobowitz]

On Tue, Jan 17, 2006 at 10:15:27PM +0200, Eli Zaretskii wrote:
> > Date: Tue, 17 Jan 2006 10:17:31 -0500
> > From: Daniel Jacobowitz <drow@false.org>
> > 
> > /space/fsf/commit/src/gdb/expprint.c:180: warning: pointer targets in passing argument 2 of 'current_language->la_printstr' differ in signedness
> 
> Is it really worth your trouble fixing this?  AFAIK, the next release
> of GCC is supposed to turn this warning off by default, so why fix
> something that ain't broken in the first place, if it's gonna bug only
> those users who have GCC 4.0.x?

Do you have a reference for this GCC change?

Also, do you think the warning is useless?  If you think it's useless,
I'll just stop, and we'll go with Mark's patch to disable it.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [ob] More warnings; Call for assistance

[Jim Blandy]

On 1/17/06, Daniel Jacobowitz <drow@false.org> wrote:
> I disagree, because (IIRC) Debian users have filed at least two bugs
> where failures in this code have led to user input crashing GDB:
>
>   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=186037
>   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=231162

Okay, sold.

> However, I'd strongly prefer to address the pointer problems first
> before returning to worry around the edges of this one :-)

I'm looking at dwarf2read.c.

Re: [ob] More warnings; Call for assistance

[Eli Zaretskii]

> Date: Tue, 17 Jan 2006 15:23:23 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Tue, Jan 17, 2006 at 10:15:27PM +0200, Eli Zaretskii wrote:
> > > Date: Tue, 17 Jan 2006 10:17:31 -0500
> > > From: Daniel Jacobowitz <drow@false.org>
> > > 
> > > /space/fsf/commit/src/gdb/expprint.c:180: warning: pointer targets in passing argument 2 of 'current_language->la_printstr' differ in signedness
> > 
> > Is it really worth your trouble fixing this?  AFAIK, the next release
> > of GCC is supposed to turn this warning off by default, so why fix
> > something that ain't broken in the first place, if it's gonna bug only
> > those users who have GCC 4.0.x?
> 
> Do you have a reference for this GCC change?

I have this:

   http://lists.gnu.org/archive/html/emacs-devel/2005-12/msg01743.html

I don't know if this counts as a reference, but it's good enough for
me ;-)

> Also, do you think the warning is useless?

I think it's too pedantic; I think it's a false alarm 99.99% of the
time.  But if you think I'm wrong, please describe situations where
this warning would point to a real trouble.

> If you think it's useless, I'll just stop, and we'll go with Mark's
> patch to disable it.

Hmm.. were you fixing only this precise warning?  I thought I've seen
other, more serious warnings as well fixed by your recent changes.

Re: [ob] More warnings; Call for assistance

[Daniel Jacobowitz]

On Tue, Jan 17, 2006 at 11:04:33PM +0200, Eli Zaretskii wrote:
> I have this:
> 
>    http://lists.gnu.org/archive/html/emacs-devel/2005-12/msg01743.html
> 
> I don't know if this counts as a reference, but it's good enough for
> me ;-)

I've seen no sign of this change in GCC.  I will follow up with them.

> > Also, do you think the warning is useless?
> 
> I think it's too pedantic; I think it's a false alarm 99.99% of the
> time.  But if you think I'm wrong, please describe situations where
> this warning would point to a real trouble.

I've fixed several places where we passed ints as var_uinteger or
unsigned ints as var_zinteger to the CLI.  They're a bit subtle to fix
(witness my earlier message today about set complaints -1).  They've
never bitten us, but they could.

> > If you think it's useless, I'll just stop, and we'll go with Mark's
> > patch to disable it.
> 
> Hmm.. were you fixing only this precise warning?  I thought I've seen
> other, more serious warnings as well fixed by your recent changes.

I fixed I believe two warnings other than this, caused by unrelated
changes in GCC.  The bulk are for this issue.

Shall we discard the remaining (more minor) cases and turn off this
warning?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [ob] More warnings; Call for assistance

[Jim Blandy]

On 1/17/06, Daniel Jacobowitz <drow@false.org> wrote:
> LA_PRINT_STRING takes a const gdb_byte * argument for the string.  But this
> is a NUL-terminated string in the debugger's memory; I think stepping back
> to char * is the best fix here.  That's the expprint.c warnings.

I wrote that up, but then I didn't like it.  The use of 'char *'
should be reserved for host-format character strings, but it looks to
me like LA_PRINT_STRING expects a string in the target format.  (Not
that the interface documentation offers much help here.)  Look at the
way c_printstr unpacks the string character by character using
extract_unsigned_integer.  Look at the way LA_EMIT_CHAR is responsible
for calling target_char_to_host.

If folks agree, then I'll make the comments in language.h clearer, and
try to finish off this change.

Re: [ob] More warnings; Call for assistance

[Eli Zaretskii]

> Date: Tue, 17 Jan 2006 16:14:55 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: gdb-patches@sourceware.org
> 
> I've fixed several places where we passed ints as var_uinteger or
> unsigned ints as var_zinteger to the CLI.  They're a bit subtle to fix
> (witness my earlier message today about set complaints -1).  They've
> never bitten us, but they could.

These cases indeed need fixing, since they reveal unclean code.  If
there are more of them, I think we should fix them too.  But the bulk
of these warnings are not for this kind of problems.  I especially
dislike these warnings when the pointer is passed to a library
function.

> Shall we discard the remaining (more minor) cases and turn off this
> warning?

I think so, yes.  We have better uses for our energy right now.

Re: [ob] More warnings; Call for assistance

[Daniel Jacobowitz]

On Tue, Jan 17, 2006 at 05:15:24PM -0800, Jim Blandy wrote:
> On 1/17/06, Daniel Jacobowitz <drow@false.org> wrote:
> > LA_PRINT_STRING takes a const gdb_byte * argument for the string.  But this
> > is a NUL-terminated string in the debugger's memory; I think stepping back
> > to char * is the best fix here.  That's the expprint.c warnings.
> 
> I wrote that up, but then I didn't like it.  The use of 'char *'
> should be reserved for host-format character strings, but it looks to
> me like LA_PRINT_STRING expects a string in the target format.  (Not
> that the interface documentation offers much help here.)  Look at the
> way c_printstr unpacks the string character by character using
> extract_unsigned_integer.  Look at the way LA_EMIT_CHAR is responsible
> for calling target_char_to_host.
> 
> If folks agree, then I'll make the comments in language.h clearer, and
> try to finish off this change.

Makes sense to me.  That requires messing with the type of the char in
the expression union (or just dropping it when we drop the warning,
which is fine by me too - we're well out into the swamps at this
point).

-- 
Daniel Jacobowitz
CodeSourcery

Re: [ob] More warnings; Call for assistance

[Daniel Jacobowitz]

On Tue, Jan 17, 2006 at 10:11:12PM +0200, Eli Zaretskii wrote:
> > Date: Tue, 17 Jan 2006 14:46:24 -0500
> > From: Daniel Jacobowitz <drow@false.org>
> > 
> > I disagree, because (IIRC) Debian users have filed at least two bugs
> > where failures in this code have led to user input crashing GDB:
> > 
> >   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=186037
> >   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=231162
> 
> These aren't crashes, GDB prints a legible error message and returns
> to top level.  However, internal_error is not the best idea for these
> situations, so we probably should arrange for GDB to mutter something
> about possibly bad format instead, and not to ask whether to dump
> core.

I suppose that would work.  And be a good idea if we're not going to
make any other changes here, along with -Wformat-nonliteral.

For reference, it'd actually be "removing -Wformat-nonliteral".  An
earlier message in this thread confused me about what that option did.

> > The advantages of static format checking, in the face of that, are
> > pretty large.
> 
> No matter what you do, as long as users are typing format strings, it
> will always be possible for a format to exceed our wildest
> imagination.  You cannot beat that, unless you are willing to
> artificially restrict users to some safe and/or easily parsable subset
> of valid formats, which I think we shouldn't do.
> 
> > avoid the use of standard printf
> 
> Avoid? how? are you saying that we should write our own version of
> printf?

Certainly we shouldn't write a new one - but the advantage of being a
GNU project and licensed under the GPL is that there's already at least
two we can choose from, probably more :-)

Another major advantage of having our own implementation of printf is that
we could document which format specifiers the GDB "printf" command
supports.  Right now we say "it's the same as if your program had
called printf", which is not necessarily true, e.g. with remote
debugging.

This is what I'd been vaguely planning on doing, but never gotten
around to.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [ob] More warnings; Call for assistance

[Daniel Jacobowitz]

On Tue, Jan 17, 2006 at 10:21:56AM -0500, Daniel Jacobowitz wrote:
> On Tue, Jan 17, 2006 at 10:17:31AM -0500, Daniel Jacobowitz wrote:
> > @@ -161,7 +161,7 @@ find_complaint (struct complaints *compl
> >     before we stop whining about it?  Default is no whining at all,
> >     since so many systems have ill-constructed symbol files.  */
> >  
> > -static unsigned int stop_whining = 0;
> > +static int stop_whining = 0;
> >  
> >  /* Print a complaint, and link the complaint block into a chain for
> >     later handling.  */
> 
> Oops!  I did check that this was correct within the logic of the file,
> but it still causes three test failures.  gdb.cp/maint.exp does
> "set complaints -1" and expects to get all complaints.
> 
> This is a var_zinteger, which is supposed to be signed - thus my
> change.  What maint.exp is doing is undocumented, and seems passingly
> illogical to me; does anyone object to my changing it to "set
> complaints 1000"?
> 
> The manual says:
> 
> `set complaints LIMIT'
>      Permits GDB to output LIMIT complaints about each type of unusual
>      symbols before becoming silent about the problem.  Set LIMIT to
>      zero to suppress all complaints; set it to a large number to
>      prevent complaints from being suppressed.

There were no objections, so I've checked this in.

-- 
Daniel Jacobowitz
CodeSourcery

2006-01-20  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.cp/maint.exp: Set complaints to a positive value.

Index: testsuite/gdb.cp/maint.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/maint.exp,v
retrieving revision 1.3
diff -u -p -r1.3 maint.exp
--- testsuite/gdb.cp/maint.exp	11 Feb 2004 14:01:25 -0000	1.3
+++ testsuite/gdb.cp/maint.exp	20 Jan 2006 23:07:52 -0000
@@ -1,4 +1,4 @@
-# Copyright 2003, 2004 Free Software Foundation Inc.
+# Copyright 2003, 2004, 2006 Free Software Foundation Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -59,7 +59,7 @@ proc test_first_component {} {
     # The function in question might complain; make sure that we see
     # all complaints.
 
-    gdb_test "set complaints -1" ""
+    gdb_test "set complaints 1000" ""
 
     test_single_component "foo"
     test_single_component "operator<<"

Re: [ob] More warnings; Call for assistance

[Eli Zaretskii]

> Date: Fri, 20 Jan 2006 18:06:41 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> > > avoid the use of standard printf
> > 
> > Avoid? how? are you saying that we should write our own version of
> > printf?
> 
> Certainly we shouldn't write a new one - but the advantage of being a
> GNU project and licensed under the GPL is that there's already at least
> two we can choose from, probably more :-)

Doing so would probably get us also the possible bugs of those
implementations, but that's a minor issue.

A more important issue is how do we convince ourselves that the
implementation we use does not blow up in some cases as well, just in
different ones?  `printf' implementations are traditionally reckless
about bad format strings.  Are the ones you suggest any better?

Re: [ob] More warnings; Call for assistance

[Daniel Jacobowitz]

On Sat, Jan 21, 2006 at 12:07:24PM +0200, Eli Zaretskii wrote:
> > Date: Fri, 20 Jan 2006 18:06:41 -0500
> > From: Daniel Jacobowitz <drow@false.org>
> > 
> > > > avoid the use of standard printf
> > > 
> > > Avoid? how? are you saying that we should write our own version of
> > > printf?
> > 
> > Certainly we shouldn't write a new one - but the advantage of being a
> > GNU project and licensed under the GPL is that there's already at least
> > two we can choose from, probably more :-)
> 
> Doing so would probably get us also the possible bugs of those
> implementations, but that's a minor issue.
> 
> A more important issue is how do we convince ourselves that the
> implementation we use does not blow up in some cases as well, just in
> different ones?  `printf' implementations are traditionally reckless
> about bad format strings.  Are the ones you suggest any better?

As far as I know, yes - for instance, newlib's seems to be careless
only in that it does sensible things for meaningless format flags.
The big difference between calling printf and having our own
implementation is that we could decode the GDB arguments - whose
types we know - at exactly the time they are consumed by printf.

Anyway, I'm not sure it's worth it or not.

-- 
Daniel Jacobowitz
CodeSourcery