two questions about GDB coding standards

two questions about GDB coding standards

[Wu Zhou]

Hello all,

I am now reading GDB coding standards(section 13.4 of GDB Internals)
and have two questions as follow.  Maybe someone of you could clarify
for me.  Thanks in advance. 

1. In section 13.4.2(memory management), it is said that gdb uses the 
string function xstrdup and the print function xasprintf.  But I saw 
in the source codes that most places use sprintf instead.  Besides 
this in-consistence, we also know that sprintf is very prone to buffer
overflow error.  So my question here is whether sprintf should be 
replaced by xasprintf here or is there any plan to achieve this? 

2. In section 13.4.3(compiler warnings), it is said that: due to the 
way that gdb is implemented most functions have unused parameters.  
Consequently the warning "-Wunused-parameter" is precluded from the 
list.  What is the way it refered in this context?  BTW, I also find
there are many unused variables in gdb source codes by using a static 
analysis tool.  But there is no "-Wunused-variable" in compiler option
to detect this.  My question here is: should this kind of unused
variables be detected or furthermore be removed out of the source to 
make it more readable?  The coding standards doesn't say anything 
about this. 

Thanks for your attention. 

Cheers
- Wu Zhou

Re: two questions about GDB coding standards

[Eli Zaretskii]

> Date: Tue, 17 May 2005 11:13:21 -0400
> From: Wu Zhou <woodzltc@cn.ibm.com>
> 
> 1. In section 13.4.2(memory management), it is said that gdb uses the 
> string function xstrdup and the print function xasprintf.  But I saw 
> in the source codes that most places use sprintf instead.

This is just a matter of converting the uses of sprintf into safer
code using xstrdup, xasprintf, xsprintf, etc.  Volunteers are welcome.

> Besides this in-consistence, we also know that sprintf is very prone
> to buffer overflow error.

That is precisely why we don't want to use it.

Re: two questions about GDB coding standards

[Mark Kettenis]

   Date: Tue, 17 May 2005 11:13:21 -0400
   From: Wu Zhou <woodzltc@cn.ibm.com>

   Hello all,

   I am now reading GDB coding standards(section 13.4 of GDB Internals)
   and have two questions as follow.  Maybe someone of you could clarify
   for me.  Thanks in advance. 

   1. In section 13.4.2(memory management), it is said that gdb uses the 
   string function xstrdup and the print function xasprintf.  But I saw 
   in the source codes that most places use sprintf instead.  Besides 
   this in-consistence, we also know that sprintf is very prone to buffer
   overflow error.  So my question here is whether sprintf should be 
   replaced by xasprintf here or is there any plan to achieve this? 

As OpenBSD warns these days:

"sprintf() is often misused, please use snprintf()"

The usage of sprintf is mainly historic, and a bad example that
shouldn't be followed.  Patches to get rid of it are welcome, and
mostly obvious ;-).  Whether xasprintf is the right replacement,
depends on the context.  In some cases the usage of xsnprintf might be
more appropriate.  The later is a version of snprintf that checks
whether the generated string fits in the buffer.

Hmm.  Perhaps I should change the aforementioned paragraph.

   2. In section 13.4.3(compiler warnings), it is said that: due to the 
   way that gdb is implemented most functions have unused parameters.  
   Consequently the warning "-Wunused-parameter" is precluded from the 
   list.  What is the way it refered in this context?  BTW, I also find
   there are many unused variables in gdb source codes by using a static 
   analysis tool.  But there is no "-Wunused-variable" in compiler option
   to detect this.  My question here is: should this kind of unused
   variables be detected or furthermore be removed out of the source to 
   make it more readable?  The coding standards doesn't say anything 
   about this. 

As configure.ac says, -Wunused-variable is up for debate. Looks like
you just started that debate ;-).  But before we enable it someone
will have to do the dirty work of finding most of the unused
variables, such that compiling with -Werror keeps being possible.
Elimination of unused variables is defenitely obvious, but watch out
for tricky #ifdef's.

Mark

Re: two questions about GDB coding standards

[Wu Zhou]

Quoting Eli Zaretskii <eliz@gnu.org>:

> > Date: Tue, 17 May 2005 11:13:21 -0400
> > From: Wu Zhou <woodzltc@cn.ibm.com>
> > 
> > 1. In section 13.4.2(memory management), it is said that gdb uses the 
> > string function xstrdup and the print function xasprintf.  But I saw 
> > in the source codes that most places use sprintf instead.
> 
> This is just a matter of converting the uses of sprintf into safer
> code using xstrdup, xasprintf, xsprintf, etc.  Volunteers are welcome.

I'd like to contribute.  But running splint against GDB source tree, I
find over 230 usages of sprintf.  If the conversion depends completely
on hand work, the workload might be quite big.  Is there any method to
achieve this more quickly?  to say, a script to handle it automatically,
or any others?  


Cheers
- Wu Zhou

Re: two questions about GDB coding standards

[Mark Kettenis]

   Date: Wed, 18 May 2005 05:51:20 -0400
   From: Wu Zhou <woodzltc@cn.ibm.com>

   Quoting Eli Zaretskii <eliz@gnu.org>:

   > > Date: Tue, 17 May 2005 11:13:21 -0400
   > > From: Wu Zhou <woodzltc@cn.ibm.com>
   > > 
   > > 1. In section 13.4.2(memory management), it is said that gdb uses the 
   > > string function xstrdup and the print function xasprintf.  But I saw 
   > > in the source codes that most places use sprintf instead.
   > 
   > This is just a matter of converting the uses of sprintf into safer
   > code using xstrdup, xasprintf, xsprintf, etc.  Volunteers are welcome.

   I'd like to contribute.  But running splint against GDB source tree, I
   find over 230 usages of sprintf.  If the conversion depends completely
   on hand work, the workload might be quite big.  Is there any method to
   achieve this more quickly?  to say, a script to handle it automatically,
   or any others?  

I'm afraid not; but hey you don't have to do all of them ;-).  Start
with the ones in code that you're a bit familliar with, and see where
you end up.  Even if you only convert 10% it is very useful.

Cheers,

Mark

Re: two questions about GDB coding standards

[Wu Zhou]

Quoting Mark Kettenis <mark.kettenis@xs4all.nl>:

> The usage of sprintf is mainly historic, and a bad example that
> shouldn't be followed.  Patches to get rid of it are welcome, and
> mostly obvious ;-).  Whether xasprintf is the right replacement,
> depends on the context.  In some cases the usage of xsnprintf might be
> more appropriate.  The later is a version of snprintf that checks
> whether the generated string fits in the buffer.

Where can I find any document about xasprintf or xsnprintf?  I can find 
xasprintf's definition in the source tree of libiberty, but didn't find
any definition for xsnprintf.  Where is it defined then?   

> As configure.ac says, -Wunused-variable is up for debate. Looks like
> you just started that debate ;-).  But before we enable it someone
> will have to do the dirty work of finding most of the unused
> variables, such that compiling with -Werror keeps being possible.
> Elimination of unused variables is defenitely obvious, but watch out
> for tricky #ifdef's.
 
Sorry I didn't notice that this is mentioned in configure.ac.  And my 
intention was also not to start debate.  But I _do_ think that removing 
these unused variables might be achievable.  My initial idea is to use
static analysis tool to detect unused variables first.  Then double check 
to eliminate these false negatives. and then use a script to remove the 
definition automatically.  What is your thought on this?  Please review
and comment.  Thanks. 


Cheers
- Wu Zhou

Re: two questions about GDB coding standards

[Eli Zaretskii]

> Date: Wed, 18 May 2005 06:24:41 -0400
> From: Wu Zhou <woodzltc@cn.ibm.com>
> Cc: gdb-patches@sources.redhat.com
> 
> Where can I find any document about xasprintf or xsnprintf?  I can find 
> xasprintf's definition in the source tree of libiberty, but didn't find
> any definition for xsnprintf.  Where is it defined then?   

Both xasprintf and xsnprintf are defined in gdb/utils.c.  There's no
documentation for them except in utils.c (code and comments).

(I think we should have a section in gdbint.texinfo that describes
useful utility functions such as these.  Feel free to write up such
documentation; I'd be glad to review it, if you do.)

Re: two questions about GDB coding standards

[Wu Zhou]

Quoting Mark Kettenis <mark.kettenis@xs4all.nl>:

>    I'd like to contribute.  But running splint against GDB source tree, I
>    find over 230 usages of sprintf.  If the conversion depends completely
>    on hand work, the workload might be quite big.  Is there any method to
>    achieve this more quickly?  to say, a script to handle it automatically,
>    or any others?  
> 
> I'm afraid not; but hey you don't have to do all of them ;-).  Start
> with the ones in code that you're a bit familliar with, and see where
> you end up.  Even if you only convert 10% it is very useful.

OK.  I'd like to begin with what you suggested in the near future.  

Cheers
- Wu Zhou

Re: two questions about GDB coding standards

[Wu Zhou]

Quoting Eli Zaretskii <eliz@gnu.org>:

> Both xasprintf and xsnprintf are defined in gdb/utils.c.  There's no
> documentation for them except in utils.c (code and comments).

yes. I found their definition in the latest gdb source tree.  My working
tree is a old one(6.3.0).  

> (I think we should have a section in gdbint.texinfo that describes
> useful utility functions such as these.  Feel free to write up such
> documentation; I'd be glad to review it, if you do.)

Surely I'd like to contribute, if only that you think that my english
is ok for this kind of task, :-)  I will try to draft some document
later for your review.  Thanks.  

Cheers
- Wu Zhou

Re: two questions about GDB coding standards

[Eli Zaretskii]

> Date: Thu, 19 May 2005 01:36:52 -0400
> From: Wu Zhou <woodzltc@cn.ibm.com>
> Cc: gdb-patches@sources.redhat.com
> 
> Surely I'd like to contribute, if only that you think that my english
> is ok for this kind of task, :-)

Don't worry about that, I can always fix your English with a minimal
effort.