Re: Proposal: convert function definitions to prototyped form

[Kevin Buettner <kevinb@cygnus.com>]

On Jun 2, 11:39am, David Taylor wrote:

> With any solution, either you're going to have to check the results or
> you're going to have to hand convert some of the stuff.  It's a
> tradeoff.  From what you say below, your script also requires some
> later hand conversions.

Yes.  But, it's conservative in that it won't break your builds.
(Or at least that's the intent.)

> Does your script produce substantially less that needs to be hand
> converted?

I only ran protoize on breakpoint.c to see what the problems were. 
This was the file that required the most conversion, however, so
hopefully it's a good representative sample.  For this file, protoize
converted 126 function declarations and fix-decls converted 138.  In
addition, not counting reformatting, the protoized version of
breakpoint.c would require 5 hand edits in order to fix problems
introduced in the conversion.  The fix-decls version requires 0 hand
edits.  After conversion, the protoized version of breakpoint.c
still had 15 functions which would need to be converted by hand (or
some other tool) whereas the fix-decls version had only 3.  Here's
a table to summarizing the above:

                        breakpoint.c conversion
                                              | protoize | fix-decls |
----------------------------------------------+----------+-----------+
Functions converted                           |   126    |   138     |
----------------------------------------------+----------+-----------+
Hand edits needed afterwards to fix problems  |          |           |
in the conversion process                     |     5    |     0     |
----------------------------------------------+----------+-----------+
Functions remaining to be converted           |    15    |     3     |
----------------------------------------------+----------+-----------+
Total number of hand edits required           |    20    |     3     |
----------------------------------------------+----------+-----------+

Recall that over all of the gdb sources, fix-decls converted slightly
over 5200 declarations.  I believe that there are only around 140 left
to consider.  This is somewhat higher than I would like and I may be
able to reduce it somewhat by making the script a little bit smarter. 
But I also want to be certain that no errors are introduced in the
process. 

I should note that there are many settings where protoize is a
completely acceptable (and probably better) tool.  With gdb, however,
we have a lot of developers scattered across the globe who need to be
able to deterministically apply the same transformations to their
sources in order to make merges and checkins easier.

With protoize, I think it's going be be difficult to guarantee
determinism since the results will vary depending upon which platform
you use to do the transformation.

> 	3) ``protoize'' screws up on PTR types.  E.g, in breakpoint.c, it
> 	   converted
> 
> 		static int
> 		cover_target_enable_exception_callback (arg)
> 		     PTR arg;
> 
> 	   to
> 
> 		static int
> 		cover_target_enable_exception_callback (__builtin_va_list arg)
> 
> I consider this to be a more serious problem than the other complaints
> you have.

I think the setup issue is important too.  (I don't know if it's more
serious though.)  As I mentioned in my first point (which is no longer
quoted), you have to do a configure in the source directory above gdb
in order to to properly run protoize or else it complains that it
can't find certain header files.  Also, you need to make sure that the
bfd header files are generated before you start.  None of these
problems are insurmountable, but to do things safely with protoize,
and in order to avoid polluting your sources with files that belong in
a build tree, it would be necessary for a script using protoize to
make a complete copy of the source tree somewhere else in order to do
the work.

> 	4) ``protoize'' does not reformat long argument lists.  The lists
> 	   end up entirely contained on one line.
> 
> So... adopt the same solution for it that you chose to adopt for your
> script -- run indent on the output!

Granted.  A suitably imaginative script could identify just the lines
that protoize touched and reformat those.  I don't think we want to
run indent on entire files however, at least not for this activity.

>     For more information on protoize, see the "Running protoize" page:
> 
> 	http://gcc.gnu.org/onlinedocs/gcc_2.html#SEC48
> 
>     and the "Caveats of using protoize" page:
> 
> 	http://gcc.gnu.org/onlinedocs/gcc_7.html#SEC135
> 
> I've used protoize before with good results.  It was a fairly
> substantial project, though not as big as gdb.

Okay.  Out of curiousity, did the project in question have a large
number of active developers?  (This doesn't necessarily matter, but
I think it's probably easier to manage this kind of thing when only
a handful of developers are affected.)

>     Two of my goals in creating the scripts for the PARAMS purging
>     activities was that the scripts should be 1) easy to invoke and 2)
>     require no hand editing of the output when done.  I.e, you shouldn't
>     have to edit any of the files that these scripts touch in order to fix
> 
> I trust that "3)" is:
> 
>     "be conservative; not convert something if it can't be sure
>     of getting it right".

You're right.  I should have mentioned this.

> I'd much rather hand convert more of the code than have it make a
> subtle but incorrect change.

I agree completely.

[...]

>     Finally, I should note that I've done a test build on GNU/Linux/x86
>     and had no build problems, nor did I see any regressions when I ran
>     the testsuite.
> 
>     The fix-decls script is below.  I'd be interested in finding out if
>     anyone else has a favorite script for doing this sort of thing.  Other
>     comments welcome too.
> 
> I'd be tempted to do a build before running your script; stash away the
> object files; run the script; do another build; compare the object
> files...

Good idea.  I'll have to see what gcc does to compare object files.
(I don't think a simple cmp works for ELF files.)

> I consider the lack of (prototyped) function declarations to be more
> serious "problem" than the use of old style function definitions in
> old code.  I'd like to see:
> 
> . declarations near the top of every file (i.e., before the first
>   function definition) for every static function in the file.
> 
> . a declaration in an included header file for every non static
>   function definition and for every non static function used.
> 
> . changes to the default configuration to "encourage" developers to
>   include the above declarations.

I don't disagree, but I think that adding prototypes for everything
should be a separate activity.  The question is whether it should
occur before the activity covered by my proposal.  And if it should
occur before, once again, we'll need to find a way to help automate
the process because if we attempt to do it incrementally by hand
over a period of time, it is likely that it'll never get done.

Kevin