Proposal: convert function definitions to prototyped form

Proposal: convert function definitions to prototyped form

[Kevin Buettner]

As many of you know, I'm in the midst of purging the use of ``PARAMS''
in prototyped function declarations from the gdb sources.  After this
activity is concluded, I'd like to begin converting function
definitions whose parameters are declared using the traditional C
style to the ISO C prototyped form.  I.e, I'd like to convert
functions of the form

    int
    foo (a, b, c)
         int a;
         char *b;
         int *c;
    {
      ...
    }

to

    int foo (int a, char *b, int *c)
    {
      ...
    }

To this end, I've been working on a script to automate most of this
conversion.  I've appended a work-in-progress version to the end of
this message and will discuss it in some detail further below.

But first...  it's been pointed out to me that the ``protoize''
program is supposed to do the same thing.  I gave ``protoize'' a whirl
and noticed the following problems:

    1) A substantial amount of initial setup is involved.  Only in
       the simplest cases could you just feed a list of files to
       work on to protoize and expect it to work okay.  In particular,
       for gdb, I found it necessary to do a configure *in the source
       directory* so that xm.h, tm.h, and nm.h are set up.  I also
       found it necessary to build in bfd so that certain header
       files are created.

       The reason all of this is necessary is because protoize invokes
       gcc to learn about the program.  As a consequence, it is also
       important to know the set of -I switches that one normally feeds
       to gcc to do a build.

    2) ``protoize'' fails to convert functions disabled by ifdefs for
       the given platform.  OTOH, on some other platform(s), these
       functions might not be disabled and would be converted.  E.g,
       in breakpoint.c, on my GNU/Linux/x86 machine, protoize failed
       to convert create_longjmp_breakpoint() which is protected by an
       "#ifdef GET_LONGJMP_TARGET".

    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)

    4) ``protoize'' does not reformat long argument lists.  The lists
       end up entirely contained on one line.

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

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
errors so that you can build again.  OTOH, the script may leave
certain portions of the file alone that could possibly have be
converted had the script been a little smarter.  The things that the
script fails to convert *will* have to be fixed later on (in order to
complete the cleanup activity), either by another script, or by hand. 
For the PARAMS purging activity, I have spent a fair amount of time
examining the diffs to make sure that this is indeed the case.  (And
I intend to do the same for the activity in this proposal.)

The reason that it is so important to avoid any hand edits is that we
want people who have local repositories or checked out source trees to
be able to run these conversion scripts against them so that merges
will be less painful.  (Even easy.)

With that said, keeping in mind the problems I noted above, I conclude
that ``protoize'' is not the appropriate tool for us to use to convert
function definitions in the gdb sources to their prototyped form.

Now, I'll tell you about the fix-decls script (see below).

The fix-decls script is run via "fix-decls <dirname>" where <dirname>
is the name of the directory to (recursively) fix.  It only touches
the .c files, with the exception of gnu-regex.c and the testsuite
files.  After it converts a traditional C style declaration to a
prototyped one, it invokes GNU indent (on the function declaration
only) to make sure that our indentation standards are still adhered
to.  There is no gratuitous reformatting of any other code.

Here are a list of things that it won't handle (at the moment,
anyway):

    - declarations with comments in them
    - function pointers
    - array declarations
    - malformed declarations (e.g, look at m88k_skip_prologue in m88k-tdep.c)

Functions with traditional C declarations with any of the above in
them will be left alone.  (And need to be converted by hand at some
point.)  But this isn't so bad.  When I ran the script over the gdb
sources, it converted 5,204 function definitions.  After throwing
out gnu-regex.c (which needs to track glibc), all of the stuff in
the testsuite, and everything in tui (which has ifdefs around each
and every function declaration), there are only 143 cases left to
examine by hand.  I've sampled these and many of them are unhandled
due to having comments following a parameter's type declaration.
If I could figure out what to do about comments, we'd have even
fewer cases to fix up by hand.

Here are some of the more interesting cases that it does handle:

Missing int in declaration (i960-tdep.c):

 CORE_ADDR
-frame_args_address (fi, must_be_correct)
-     struct frame_info *fi;
+frame_args_address (struct frame_info *fi, int must_be_correct)
 {

Comma separated parameter lists (remote-mm.c):

 static void
-init_target_mm (tstart, tend, dstart, dend, entry, ms_size, rs_size, arg_start)
-     ADDR32 tstart, tend, dstart, dend, entry;
-     INT32 ms_size, rs_size;
-     ADDR32 arg_start;
+init_target_mm (ADDR32 tstart, ADDR32 tend, ADDR32 dstart, ADDR32 dend,
+		ADDR32 entry, INT32 ms_size, INT32 rs_size, ADDR32 arg_start)
 {

Comma separated list with differing number of stars on the parameter
names (sparc-tdep.c):

 static branch_type
-isbranch (instruction, addr, target)
-     long instruction;
-     CORE_ADDR addr, *target;
+isbranch (long instruction, CORE_ADDR addr, CORE_ADDR * target)
 {

And another one (command.c):

 struct cmd_list_element *
-lookup_cmd_1 (text, clist, result_list, ignore_help_classes)
-     char **text;
-     struct cmd_list_element *clist, **result_list;
-     int ignore_help_classes;
+lookup_cmd_1 (char **text, struct cmd_list_element *clist,
+	      struct cmd_list_element **result_list, int ignore_help_classes)
 {

Out of order comma separated parameter list - note positions of ``from''
and ``to'' (gdbserver/remote-utils.c):

 void
-decode_M_packet (from, mem_addr_ptr, len_ptr, to)
-     char *from, *to;
-     CORE_ADDR *mem_addr_ptr;
-     unsigned int *len_ptr;
+decode_M_packet (char *from, CORE_ADDR * mem_addr_ptr, unsigned int *len_ptr,
+		 char *to)
 {

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.

--- fix-decls ---
#!/usr/bin/perl -w

use File::Find;
use FileHandle;
use IPC::Open3;
use English;

my ($root) = @ARGV;

if (!defined($root)) {
    die "Usage: $0 root\n";
}

@ARGV = ();

find(
    sub { 
	if ($_ eq 'testsuite') {
	    $File::Find::prune = 1;
	} elsif (-f && -T && /\.c$/ && $_ ne "gnu-regex.c") {
	    push @ARGV, $File::Find::name;
	}
    },
    $root
);

#$INPLACE_EDIT = '.bak';
$INPLACE_EDIT = '';
undef $/;			# slurp entire files

while (<>) {
    s/
	^			# line start
	( 
	  \w+			# function name
	)
	(
	  \s*			# spaces
	  \(			# left paren
	  \s*			# spaces
	)
	(
	  (?:\w+\s*,\s*)*	# 1 thru N-1 parameter names
	  \w+			# last parameter name
	)
	(
	  \s*			# spaces
	  \)			# right paren
	)
	(
	  (?:
	    [^;{}()]+;		# trad C parameter decl
	  )*
	)
	(
	  \s*			# spaces
	)
	(?=^{)			# lookahead to make sure we see a
				# right curly brace at the beginning
				# of the line
    /
	fix_decl($1, $2, $3, $4, $5, $6);
    /smgex;

    s/
	^			# line start
	( 
	  \w+			# function name
	)
	\s*			# spaces
	\(			# left paren
	\s*			# spaces
	\)			# right paren
	(?=\s*^{)		# lookahead to make sure we see a
				# right curly brace at the beginning
				# of the line
     /$1 (void)/smgx;

    print;
}

sub fix_decl {
    my ($funcname, $lparen, $params, $rparen, $decls, $spaces) = @_;
    my %h = ();
    my ($param, $decl);

    # Define $bailstr to be the original function declaration.  We
    # return it when we see something which doesn't make sense.
    my $bailstr = $funcname . $lparen . $params . $rparen . $decls . $spaces;

    if ($funcname =~ /^(do|while|if)$/) {
	# 'if', 'do', and 'while' are not function names
	# find_overload_match() in valops.c contains an if statement
	# which is confused as a function if we don't have this test.
	return $bailstr;
    }

    foreach $param (split /\s*,\s*/, $params) {
	if (defined $h{$param} || $param eq 'void') {
	    # Bail; either param has already been encountered or
	    # it's void in which case the decl in question is already
	    # ISO C.
	    return $bailstr;
	}
	$h{$param} = "int $param";	# Default
    }

    $decls =~ s/\s*;\Z//;		# remove final semicolon
    			
    foreach $decl (split /\s*;\s*/, $decls) {
	my ($type, $dparams) = 
	    $decl =~ /^				# beginning of string
	              (.*?)			# type
	              (				# dparams...
		        (?:
			  \**			#  stars
			  \s*			#  spaces
			  \w+			#  identifier
			  \s*			#  spaces
			  ,			#  comma
			  \s*			#  spaces
			)*			# any number of the above
			\**			#  stars
			\s*			#  spaces
			\w+			#  identifier
		      )
		      $				# end of string
		     /sx;
	return $bailstr			if !defined $type || !defined $dparams;
	$type =~ s/\A\s+//;		# nuke leading spaces
	$type =~ s/\s+\Z//;		# nuke trailing spaces
	return $bailstr			if $type eq '';
					# Bail if no type
	foreach $param (split /\s*,\s*/, $dparams) {

	    my ($stars, $stripped_param) = 
		$param =~ /(\**)\s*(\w+)/;

	    if (!defined($stripped_param) || !defined $h{$stripped_param}) {
		# Either we couldn't find the parameter or else
		# the parameter wasn't found in the parameter list
		return $bailstr;
	    }
	    $h{$stripped_param} = "$type $param";
	}
    }

    my $newparams = join(', ', map { $h{$_} } split(/\s*,\s*/, $params));

    my $newdecl = reindent("$funcname ($newparams)\n{\n}\n");
    $newdecl =~ s/{\n}//;
    return $newdecl;
}


sub reindent {
    my ($decl, $line_length) = @_;
    $line_length = 80		unless defined $line_length;
    my ($rfh, $wfh, $efh) = (FileHandle->new, FileHandle->new,
					      FileHandle->new);
    my $pid = open3($wfh, $rfh, $efh, "indent -l$line_length");
    $rfh->input_record_separator(undef);
    $efh->input_record_separator(undef);
    $wfh->print($decl);
    $wfh->close();
    my $replacement = <$rfh>;
    $rfh->close();
    my $errstr = <$efh>;
    $efh->close();
    waitpid $pid, 0;
    $replacement =~ s#\n$##;
    if ($errstr ne "") {
	print STDERR "Check $ARGV...\n$errstr\nInput:$decl\nOutput:$replacement\n\n"
    }
    $replacement;
}
--- end fix-decls ---

Re: Proposal: convert function definitions to prototyped form

[Andrew Cagney]

Kevin Buettner wrote:
> 
> As many of you know, I'm in the midst of purging the use of ``PARAMS''
> in prototyped function declarations from the gdb sources.  After this
> activity is concluded, I'd like to begin converting function
> definitions whose parameters are declared using the traditional C
> style to the ISO C prototyped form.  I.e, I'd like to convert
> functions of the form
> 
>     int
>     foo (a, b, c)
>          int a;
>          char *b;
>          int *c;
>     {
>       ...
>     }
> 
> to
> 
>     int foo (int a, char *b, int *c)
>     {
>       ...
>     }

(you mean
	int
	foo (int a, char *b, int *c)
:-)

The obvious thing to note is that by moving to pure ISO-C we eliminate
the type promotion problems.  For instance:

	void foo (enum e e);
	void
	foo (e)
		enum e e;
	{
	}

is not well defined and many compilers reject it.  Plenty of similar
examples (involving char, short, and the like exist).

It is one less issue that people need to worry about.

The only concern I have is, given the slightly more complex nature of
the script (compared to PARAMS) there is a possibility that the
conversion re-orders or re-types the argument list.  With that in mind,
should a pre-cursor to this be to least have prototypes for all
(global?) functions (-Wmissing-prototypes?) or only do the conversion
when there is a prototype visible?

	Andrew

PS: You may want to add gdb/*-share to the list of directories to avoid.

Re: Proposal: convert function definitions to prototyped form

[Mark Kettenis]

   Date: Fri, 2 Jun 2000 00:50:19 -0700
   From: Kevin Buettner <kevinb@cygnus.com>

   Comma separated list with differing number of stars on the parameter
   names (sparc-tdep.c):

    static branch_type
   -isbranch (instruction, addr, target)
   -     long instruction;
   -     CORE_ADDR addr, *target;
   +isbranch (long instruction, CORE_ADDR addr, CORE_ADDR * target)
    {

I guess you should tweak it some more such that it outputs

   CORE_ADDR *target

instead of

   CORE_ADDR * target

(note the spurious space between * and target).

Mark

Re: Proposal: convert function definitions to prototyped form

[Kevin Buettner]

On Jun 2,  2:26pm, Mark Kettenis wrote:

>    Date: Fri, 2 Jun 2000 00:50:19 -0700
>    From: Kevin Buettner <kevinb@cygnus.com>
> 
>    Comma separated list with differing number of stars on the parameter
>    names (sparc-tdep.c):
> 
>     static branch_type
>    -isbranch (instruction, addr, target)
>    -     long instruction;
>    -     CORE_ADDR addr, *target;
>    +isbranch (long instruction, CORE_ADDR addr, CORE_ADDR * target)
>     {
> 
> I guess you should tweak it some more such that it outputs
> 
>    CORE_ADDR *target
> 
> instead of
> 
>    CORE_ADDR * target
> 
> (note the spurious space between * and target).

I noticed that.  The space was put there by ``indent''.  I would very
much like to get rid of that space and it would be easy to make the
perl script postprocess the ``indent'' output.  But in doing so, we
(obviously) generate different output than that of ``indent''.

I suppose the other solution is to fix indent.  :-)

FYI, I'm using GNU indent 2.2.5.

Kevin

Re: Proposal: convert function definitions to prototyped form

[J.T. Conklin]

>>>>> "Kevin" == Kevin Buettner <kevinb@cygnus.com> writes:
Kevin> I noticed that.  The space was put there by ``indent''.  I
Kevin> would very much like to get rid of that space and it would be
Kevin> easy to make the perl script postprocess the ``indent'' output.
Kevin> But in doing so, we (obviously) generate different output than
Kevin> that of ``indent''.
Kevin>
Kevin> I suppose the other solution is to fix indent.  :-)

You can tell indent about all the types defined by typedef with -T
option, and then it won't add the extra space.  It shouldn't be too
difficult to identify all the types.  

It might be useful for us to maintain an indent.pro file that has
these definitions so that additional runs of indent don't add back
the space.

        --jtc

-- 
J.T. Conklin
RedBack Networks

Re: Proposal: convert function definitions to prototyped form

[Kevin Buettner]

On Jun 2,  7:42pm, Andrew Cagney wrote:

> The only concern I have is, given the slightly more complex nature of
> the script (compared to PARAMS) there is a possibility that the
> conversion re-orders or re-types the argument list.  With that in mind,
> should a pre-cursor to this be to least have prototypes for all
> (global?) functions (-Wmissing-prototypes?) or only do the conversion
> when there is a prototype visible?

I've given this matter a lot of thought.

I agree that it would be desirable to have prototypes for all
functions.  Unfortunately, while it is easy to generate prototypes,
it's not so easy to know where to stick them.  Also, even if we had
prototypes in place, there's no guarantee that we'd catch the errors
after a few builds because I think there's some code in gdb (though I
don't know how much) that never gets built!  (Due to ifdefs and
near obsolete ports.)

What we really need is a method for vetting all of the changes
immediately after running the script.  I.e, we need to make sure that
the conversion does no reordering or retyping of any argument list.
Also, we need to make sure that the rewritten function declaration
is syntactically correct.

While examining the diffs (made with the -u switch) an idea occurred
to me.  Consider the following example:

    diff -ur ../../orig/gdb-sourceware/wrapper.c ./wrapper.c
    --- ../../orig/gdb-sourceware/wrapper.c	Sat May 27 17:10:27 2000
    +++ ./wrapper.c	Thu Jun  1 23:33:16 2000
    @@ -57,11 +57,8 @@
     static int wrap_parse_and_eval_type (char *);
     
     int
    -gdb_parse_exp_1 (stringptr, block, comma, expression)
    -     char **stringptr;
    -     struct block *block;
    -     int comma;
    -     struct expression **expression;
    +gdb_parse_exp_1 (char **stringptr, struct block *block, int comma,
    +		 struct expression **expression)
     {
       struct gdb_wrapper_arguments args;
       args.args[0].pointer = stringptr;

In the above diff, the lines prepended with `-' represent the original
K&R definition.  And the lines prepended with `+' represent the
transformed code.  Moreover, the diff is extremely regular in this
respect.  So...

If you take the lines which begin with `+', prepend the type on the
line before the `-' lines and tack a semicolon onto the end, you end
up with a prototype declaration.  And, if you take the lines beginning
with `-', again tack the type onto the front and put a function body
underneath it, you have a K&R style (traditional) function definition.

Now if you put these into a file with the prototype first and the K&R
definition later on, you can run "gcc -Wall" on it to see if any
warnings are produced.  Obviously, if we get warnings, we need to look
closer to see if something went wrong with the fix-decls conversion.

Of course, there are other details to consider, like making sure that
all of the types, structs, unions, and enums are declared.  Also,
in a source tree as big as gdb, we'll likely wind up with a number
of functions with the same name, so some method of disambiguating
these will be necessary.  And then of course, there's the matter
of no declared return type and other oddments.

I've written a script called ``check-decls'' which performs these
transformations on the diff output.  When I run it on the above diff,
it produces the following output (indented by four spaces by me for
readability)

    struct block { int f0; };
    struct expression { int f1; };
    #define INLINE
    #define private
    #define CONST const
    #define NORETURN
    void init___ (void *);
    int gdb_parse_exp_1 (char **stringptr, struct block *block, int comma,
		     struct expression **expression);

    int
    gdb_parse_exp_1 (stringptr, block, comma, expression)
	 char **stringptr;
	 struct block *block;
	 int comma;
	 struct expression **expression;
    {
      int ret;
      init___ (&ret);
      return ret;
    }

    void
    use___ (void)
    {
    }

The use___ () function isn't interesting in this example, but it would
be if there had been a static declaration.

Here's what happens when I run it on *all* of the diffs:

ocotillo:ptests$ ./check-decls <declsdiff >prog.c
ocotillo:ptests$ wc prog.c
  50235  112228  960827 prog.c
ocotillo:ptests$ gcc -c -Wall prog.c
prog.c: In function `exit':
prog.c:39303: warning: function declared `noreturn' has a `return' statement
prog.c: At top level:
prog.c:45903: parse error before `arg_type'
prog.c: In function `value_primitive_field':
prog.c:45907: declaration for parameter `arg_type' but no such parameter
prog.c:45906: declaration for parameter `fieldno' but no such parameter
prog.c:45905: declaration for parameter `offset' but no such parameter
prog.c:45904: declaration for parameter `arg1' but no such parameter
prog.c:45908: argument `arg_type' doesn't match prototype
prog.c:5886: prototype declaration
prog.c:45908: argument `arg1' doesn't match prototype
prog.c:5886: prototype declaration

The `exit' warning is due to the fact that there's a declaration and
definition of exit() from standalone.c.  It is of no concern.

The error following this warning looks more serious.  Here's the declaration
and the definition of the function involved:

    value_ptr value_primitive_field (register value_ptr arg1, int offset,
			   register int fieldno, register struct type *arg_type);

    value_ptr
    value_primitive_field (arg1, offset, fieldno, arg_type)
	 register value_ptr arg1;
	 int offset;
	 register int fieldno;
	 register struct type *arg_type;
    {
      value_ptr ret;
      init___ (&ret);
      return ret;
    }

I looked at this for a long, long time and didn't see anything wrong.
Finally, I realized that arg_type was a type from a different file.
(Which is one of the problems with throwing everything into one big
pot.)  Anyway, here's the type that the script declared:

    typedef struct t44 { int f44; } arg_type;

And here's the (transformed) definition which caused it to be defined:

    bool_t
    xdr_arg_type(xdrs, objp)
	    XDR *xdrs;
	    arg_type *objp;
    {
      bool_t ret;
      init___ (&ret);
      return ret;
    }

So it turns out that it's nothing to worry about.

And that's it.  There are no other warnings or errors.  Which means
that the transformation was successful and didn't mess up any of
the parameter types.

The check-decls script is below.  One might argue that it is about as
complex as the fix-decls script.  This is true, but the code which
actually extracts the `-' and `+' lines is fairly simple.  Also, after
being extracted, there are no transformations made to these lines
aside from appending ___<num> to the function name if the script
detects that the function name has already been seen.  Most
importantly, the parameter lists are not rewritten in any way.

Most of the complexity is in the analysis and generation of the
type, struct, enum, and union declarations.  But uniqueness of
these is easy to verify.  Plus, if something is screwed up, the
compiler complains.

--- check-decls ---
#!/usr/bin/perl -w

# Feed this script a unidiff after running fix-decls and it generates
# (on stdout) a program which may be used to test the validity of the
# conversion.  Just run the result through gcc -Wall and if it
# generates any warnings, there's a problem...

undef $/;		# slurp mode
my $diff = <>;		# read entire diff in $diff;

my $decls = '';
my $defns = '';

my %userstructs = ();
my %userenums = ();
my %usertypes = ();
my %funcnames = ();
my $funcname_gensym = 0;		# for names that clash
my @needuse;

while ($diff =~
	/ (
	    ^ 				# beginning of line
	    [^\n]+			# everything til the end of line
	  )
	  \n				# newline
	  (
	    (?:
	      ^				#   beginning of line
	      -				#   minus sign
	      (?: \n			#   either just a newline
		|			#     -- or -- 
	          [^-\n]		#   any character but minus and newline
	          [^\n]*		#   the rest of the line
	          \n    		#   including the newline
	      )
	    )+				# one or more of the above
	  )
	  (
	    (?:
	      ^				#   beginning of line
	      \+			#   plus sign
	      [^+]			#   any character but plus
	      [^\n]*			#   the rest of the line
	      \n			#   including the newline
	    )+				# one or more of the above
	  )
	                                                           /mgx) {
    my ($rettype, $traddecl, $isodecl) = ($1, $2, $3);

    # Remove leading diff character from the lines extracted
    foreach ($rettype, $traddecl, $isodecl) {
	s/^.//mg;
    }

    # Find type names in parameter list
    my $parmdecls = $traddecl;
    $parmdecls =~ s/^\w+\s*\([^)]*\)//;
    foreach my $parm (split /\s*;\s*/, $parmdecls) {
	$parm =~ s/\s*\**\w+(,|$).*$//;
	analyze_type($parm);
    }

    # Resolve collisions between function name (either due to statics
    # or due to the names being in different branches of an ifdef)
    my ($funcname) = $traddecl =~ /^(\w+)/;
    if (defined $funcnames{$funcname}) {
	foreach ($traddecl, $isodecl) {
	    s/\b$funcname\b/${funcname}___$funcname_gensym/;
	}
	$funcname .= "___$funcname_gensym";
	$funcname_gensym++;
    }
    $funcnames{$funcname} = $funcname;

    # Nuke comments in the return type
    $rettype =~ s#/\*.*?\*/##g;

    # Nuke partial comment in return type
    $rettype =~ s#^.*?\*/##;

    # Eliminate ``CALLBACK'' from return type
    $rettype =~ s/\bCALLBACK\b//;

    # Eliminate ``extern'' from return type
    $rettype =~ s/\bextern\b//;

    # Eliminate leading and trailing spaces from return type
    $rettype =~ s/^\s*//;
    $rettype =~ s/\s*$//;

    if (($rettype =~ /^#/) || ($rettype eq '')) {
	# preprocessor line or empty string
	$rettype = 'int';
    } elsif ($rettype eq "static") {
	$rettype = 'static int';
    } elsif ($rettype eq "private") {
	$rettype = 'static int';
    } else {
	analyze_type($rettype);
    }

    $isodecl =~ s/\n\Z/;\n/;

    $decls .= "$rettype $isodecl";
    if ($rettype =~ /\bvoid$/) {
	$defns .= "$rettype\n$traddecl\{\n}\n\n";
    } else {
	$defns .= "$rettype\n$traddecl\{\n  $rettype ret;\n"
	       .  "  init___ (&ret);\n  return ret;\n}\n\n";
    }

    if ($rettype =~/\bstatic\b/) {
	push @needuse, $funcname;
    }
}


my $typeidx = 0;

foreach $key (sort keys %usertypes) {
    print "typedef struct t$typeidx { int f$typeidx; } $key;\n";
    $typeidx++;
}

foreach $key (sort keys %userstructs) {
    print "$key { int f$typeidx; };\n";
    $typeidx++;
}

foreach $key (sort keys %userenums) {
    print "$key { e$typeidx };\n";
    $typeidx++;
}

print "#define INLINE\n";
print "#define private\n";
print "#define CONST const\n";
print "#define NORETURN\n";
print "void init___ (void *);\n";

print $decls;
print "\n";
print $defns;

print "void\nuse___ (void)\n{\n";
foreach (@needuse) {
    print "  init___ ($_);\n";
}
print "}\n";

sub analyze_type {
    my ($parm) = @_;
    $parm =~ s/\s*\**\s*$//;
    my $type;
    if ($parm =~ /\b(struct|union)\b/) {
	$parm =~ s/\A.*\b(struct|union)\b/$1/s;
	$parm =~ s/\s*\**\s*\Z//s;
	$userstructs{$parm} = $parm;
    } elsif ($parm =~ /\b(enum)\b/) {
	$parm =~ s/\A.*\b(enum)\b/$1/s;
	$parm =~ s/\s*\**\s*\Z//s;
	$userenums{$parm} = $parm;
    } elsif ((($type) = $parm =~ /(\w+)$/)
	&& ($type !~ /^(int|char|long|short|unsigned|double
			   |register|void|const|static)$/x)) {
	$usertypes{$type} = $type;
    }
}
--- end check-decls ---

Re: Proposal: convert function definitions to prototyped form

[Kevin Buettner]

On Jun 2,  7:42pm, Andrew Cagney wrote:

> PS: You may want to add gdb/*-share to the list of directories to avoid.

I can certainly do this, but I'd like to know why they shouldn't be
converted...

Re: Proposal: convert function definitions to prototyped form

[Andrew Cagney]

Kevin Buettner wrote:
> 
> As many of you know, I'm in the midst of purging the use of ``PARAMS''
> in prototyped function declarations from the gdb sources.  After this
> activity is concluded, I'd like to begin converting function
> definitions whose parameters are declared using the traditional C
> style to the ISO C prototyped form.  I.e, I'd like to convert
> functions of the form

Something to consider with the timing.  There are Pascal, ObjectiveC.*
and random other Apple files pending.  It might be good to wait until
the buil of the work is in the repository so that we reduce the number
of contributors / maintainers that get hit for six by this one :-)

Unlike params, this one will really hurt people with private diffs.

	Andrew

Re: Proposal: convert function definitions to prototyped form

[Andrew Cagney]

Kevin Buettner wrote:
> 
> On Jun 2,  7:42pm, Andrew Cagney wrote:
> 
> > PS: You may want to add gdb/*-share to the list of directories to avoid.
> 
> I can certainly do this, but I'd like to know why they shouldn't be
> converted...

Some of the *-share files are based on code from third parties.  We
should probably try to avoid munging that code - it will make it harder
for us merge back patches.  rdi-share comes to mind.

As you note, the TUI which will be cleaned up, can be handed separatly.

	Andrew

Re: Proposal: convert function definitions to prototyped form

[Andrew Cagney]

"J.T. Conklin" wrote:
> 
> >>>>> "Kevin" == Kevin Buettner <kevinb@cygnus.com> writes:
> Kevin> I noticed that.  The space was put there by ``indent''.  I
> Kevin> would very much like to get rid of that space and it would be
> Kevin> easy to make the perl script postprocess the ``indent'' output.
> Kevin> But in doing so, we (obviously) generate different output than
> Kevin> that of ``indent''.
> Kevin>
> Kevin> I suppose the other solution is to fix indent.  :-)
> 
> You can tell indent about all the types defined by typedef with -T
> option, and then it won't add the extra space.  It shouldn't be too
> difficult to identify all the types.
> 
> It might be useful for us to maintain an indent.pro file that has
> these definitions so that additional runs of indent don't add back
> the space.

Given that people are currently editing headers to remove spaces by hand
this sounds like a good idea.  One thing, how does one pass
``indent.pro'' rather than have it pick up ``.indent.pro''?  I suspect
it can be maintained largely by hand (please not another perl script to
maintain it - we can't make perl a requirement on developers :-)

	enjoy,
		Andrew

Re: Proposal: convert function definitions to prototyped form

[J.T. Conklin]

>>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
Andrew> Given that people are currently editing headers to remove
Andrew> spaces by hand this sounds like a good idea.  One thing, how
Andrew> does one pass ``indent.pro'' rather than have it pick up
Andrew> ``.indent.pro''?  I suspect it can be maintained largely by
Andrew> hand (please not another perl script to maintain it - we can't
Andrew> make perl a requirement on developers :-)

For what we want to do, indent needs an option to disable processing
the .indent.pro files located in the current or home directories, and
another to process an arbitrary file.  I examined the latest revision
of the indent sources, 2.2.5, and I can not find an option to process
a file containing options.

It appears that indent is being actively maintained.  Perhaps someone
with a bit of time on their hands can work with the indent maintainer
to get such an option in a future revesion.

        --jtc

-- 
J.T. Conklin
RedBack Networks

Re: Proposal: convert function definitions to prototyped form

[Eric Bachalo]

"J.T. Conklin" wrote:
> 
> >>>>> "Kevin" == Kevin Buettner <kevinb@cygnus.com> writes:
> Kevin> I noticed that.  The space was put there by ``indent''.  I
> Kevin> would very much like to get rid of that space and it would be
> Kevin> easy to make the perl script postprocess the ``indent'' output.
> Kevin> But in doing so, we (obviously) generate different output than
> Kevin> that of ``indent''.
> Kevin>
> Kevin> I suppose the other solution is to fix indent.  :-)
> 
> You can tell indent about all the types defined by typedef with -T
> option, and then it won't add the extra space.  It shouldn't be too
> difficult to identify all the types.
> 
> It might be useful for us to maintain an indent.pro file that has
> these definitions so that additional runs of indent don't add back
> the space.

I believe etags does a fairly good job a finding typedefs.  I would be a
fairly simple script to parse the TAGS file to make a list of all typedefs.  I
have a python script for this.  Not sure how ported etags and python are for
all the hosts that compile GDB though. 

- Eric Bachalo

Re: Proposal: convert function definitions to prototyped form

[Andrew Cagney]

Eric Bachalo wrote:
> 
> "J.T. Conklin" wrote:
> >
> > >>>>> "Kevin" == Kevin Buettner <kevinb@cygnus.com> writes:
> > Kevin> I noticed that.  The space was put there by ``indent''.  I
> > Kevin> would very much like to get rid of that space and it would be
> > Kevin> easy to make the perl script postprocess the ``indent'' output.
> > Kevin> But in doing so, we (obviously) generate different output than
> > Kevin> that of ``indent''.
> > Kevin>
> > Kevin> I suppose the other solution is to fix indent.  :-)
> >
> > You can tell indent about all the types defined by typedef with -T
> > option, and then it won't add the extra space.  It shouldn't be too
> > difficult to identify all the types.
> >
> > It might be useful for us to maintain an indent.pro file that has
> > these definitions so that additional runs of indent don't add back
> > the space.
> 
> I believe etags does a fairly good job a finding typedefs.  I would be a
> fairly simple script to parse the TAGS file to make a list of all typedefs.  I
> have a python script for this.  Not sure how ported etags and python are for
> all the hosts that compile GDB though.

As a guide, check the list in:

http://www.gnu.ai.mit.edu/prep/standards_45.html

	Andrew

Re: Proposal: convert function definitions to prototyped form

[Daniel Berlin]

> > I believe etags does a fairly good job a finding typedefs.  I would be a
> > fairly simple script to parse the TAGS file to make a list of all typedefs.  I
> > have a python script for this.  Not sure how ported etags and python are for
> > all the hosts that compile GDB though.
> 
> As a guide, check the list in:
> 
> http://www.gnu.ai.mit.edu/prep/standards_45.html
> 
> 	Andrew
> 
Funny you guys bring up the number of types in GDB.
According to the debug info, there are >82,000.
How exactly are we counting them, again?

Re: Proposal: convert function definitions to prototyped form

[Andrew Cagney]

Kevin Buettner wrote:
> 
> As many of you know, I'm in the midst of purging the use of ``PARAMS''
> in prototyped function declarations from the gdb sources.  After this
> activity is concluded, I'd like to begin converting function
> definitions whose parameters are declared using the traditional C
> style to the ISO C prototyped form.  I.e, I'd like to convert
> functions of the form

Kevin,

Would a fair summary of this be that you've encountered the following
road blocks:

	o	indent.pro

		So that indent gives better results


	o	a drain of some of the backlog of
		patches (pascal I believe is now
		going in).

		Apple and HP?

No one has objected to the principal (well not on gdb-patches :-) and
the tool would be based on perl rather than the the gcc thing.

Once those blockages are cleared, it can be scheduled and can go
through?

	Andrew

Re: Proposal: convert function definitions to prototyped form

[Stan Shebs]

Daniel Berlin wrote:
> 
> Funny you guys bring up the number of types in GDB.
> According to the debug info, there are >82,000.
> How exactly are we counting them, again?

Since GDB sources are around 400K lines or so, it's highly unlikely
that there is a type definition for every five lines in the
C sources.  You're seeing duplicates in the debug info, presumably.

Stan

Re: Proposal: convert function definitions to prototyped form

[Daniel Berlin]

Before my patches to eliminate duplicate dwarf2 info, it said there were
120k. 
So something isn't adding up here. 
I find it hard to believe we duplicate that much info.
It's pretty much absurd.
--Dan

On Mon, 12 Jun 2000, Stan Shebs wrote:

> Daniel Berlin wrote:
> > 
> > Funny you guys bring up the number of types in GDB.
> > According to the debug info, there are >82,000.
> > How exactly are we counting them, again?
> 
> Since GDB sources are around 400K lines or so, it's highly unlikely
> that there is a type definition for every five lines in the
> C sources.  You're seeing duplicates in the debug info, presumably.
> 
> Stan
> 

Re: Proposal: convert function definitions to prototyped form

[Kevin Buettner]

On Jun 12,  4:30pm, Eric Bachalo wrote:

> > You can tell indent about all the types defined by typedef with -T
> > option, and then it won't add the extra space.  It shouldn't be too
> > difficult to identify all the types.
> > 
> > It might be useful for us to maintain an indent.pro file that has
> > these definitions so that additional runs of indent don't add back
> > the space.
> 
> I believe etags does a fairly good job a finding typedefs.  I would be a
> fairly simple script to parse the TAGS file to make a list of all typedefs.  I
> have a python script for this.  Not sure how ported etags and python are for
> all the hosts that compile GDB though. 

What you suggest would work so long as you also run etags over the
system header files.

For the task at hand, however, the check-decls script has already
identified the necessary types.  (Recall that check-decls is run on
the "diff -u" output after doing a fix-decls.  It constructs a program
which is then compiled with "gcc -Wall" to check the validity of the
protoization performed by fix-decls.  In the process of constructing
the validating program, it must first identify all of the types
requiring typedefs.)

Here is the list of types found by check-decls:

    ADDR32, B_TYPE, COMMON_ENTRY_PTR, CORE_ADDR, CPUSpace, DCACHE,
    DIE_REF, DOUBLEST, EXTR, EventRecord, FDR, FILE, HWND, INSN_WORD,
    INT32, LONG, LONGEST, LPARAM, LRESULT, PDR, PTR, PTRACE_ARG3_TYPE,
    PXDB_header_ptr, Point, Ptrace_return, RDB_EVENT, REGISTER_TYPE,
    RgnHandle, Rptrace, SAVED_BF_PTR, SAVED_F77_COMMON_PTR,
    SAVED_FUNCTION, SYMR, TTRACE_ARG_TYPE, UINT, ULONGEST, WAITTYPE,
    WPARAM, WindowPtr, XDR, YYSTYPE, alpha_extra_func_info_t,
    arg_array, arg_one, arg_type, arg_value, argsin, asection,
    attach_continue_t, bfd, bfd_arch_info_type, bfd_byte,
    bfd_signed_vma, bfd_vma, bool_t, boolean, boolean_t, bpstat,
    branch_type, catch_errors_ftype, catch_fork_kind, cma__t_int_tcb,
    disassemble_info, dld_cache_t, dnttpointer, dst_rec_ptr_t,
    dst_sec, dst_sect_ref_t, dst_type_t, file_ptr, fltset_t,
    fpregset_t, func_call, gdb_client_data, gdb_fpregset_t,
    gdb_gregset_t, gdb_thread_t, gdb_threadref, gregset_t,
    host_callback, insertion_state_t, insn_type, kern_return_t,
    lwpid_t, mach_msg_header_t, mach_msg_id_t, mach_msg_type_name_t,
    mach_port_mscount_t, mach_port_t, memory_page_t, memxferfunc,
    mips_extra_func_info_t, namespace_enum, off_t, pid_t,
    port_chain_t, process_state_t, procinfo, quick_file_entry,
    quick_module_entry, quick_procedure_entry, return_mask,
    rmt_thread_action, sec_ptr, serial_t, serial_ttystate, sigset_t,
    size_t, sltpointer, stepping_mode_t, sysset_t, t_inst, task_t,
    td_err_e, td_thr_state_e, td_thr_type_e, td_thragent_t,
    td_thrhandle_t, thread_array_t, thread_info, thread_t, threadinfo,
    threadref, time_t, tree, ttevents_t, ttreq_t, ttstate_t, ttwopt_t,
    u_long, ui_file_delete_ftype, ui_file_flush_ftype,
    ui_file_fputs_ftype, ui_file_isatty_ftype, ui_file_put_ftype,
    ui_file_rewind_ftype, va_list, value_ptr, and xdrproc_t

Note that there are likely typedefs for types in the gdb sources
missing from the above list, but these were not necessary for doing
the fix-decls conversion.  Also, there are some in the above list
which are defined in system header files.  But that's okay because
indent still needs to know about them in order to do a good job of
formatting.

Re: Proposal: convert function definitions to prototyped form

[Anatoly Vorobey]

On Mon, Jun 12, 2000 at 06:23:27PM -0700, Daniel Berlin wrote:
> Before my patches to eliminate duplicate dwarf2 info, it said there were
> 120k. 
> So something isn't adding up here. 

How do you eliminate them? The vast majority of types in debug info
are duplicate header file types coming from different source files.
Reducing this should cut the number much more drastically than
from 120k to 80k. I'd say by factor of 8 at the very least. 

My code which reads stabs info says that there are 52276 types (not
trying to remove duplicates) defined
in gdb (not including libbfd, libiberty and libopcodes; including them
gives another 13000 or so) and of them 886 unique type *names*. This
gives a lower bound, obviously, since names may be reused or may denote
different types even coming from the same header due to different defines --
but it's also more useful than the raw number since it doesn't count the
multitude of unnamed automatically generated types which are pointers, 
aliases, forward references, etc.

That would mean around one type name per 400 lines of code. Since almost all
type declarations are in header files which are about 1/10 of all sources
in size, it looks reasonable. 

-- 
Anatoly Vorobey,
mellon@pobox.com http://pobox.com/~mellon/
"Angels can fly because they take themselves lightly" - G.K.Chesterton

Re: Proposal: convert function definitions to prototyped form

[Daniel Berlin]

On Tue, 13 Jun 2000, Anatoly Vorobey wrote:

> On Mon, Jun 12, 2000 at 06:23:27PM -0700, Daniel Berlin wrote:
> > Before my patches to eliminate duplicate dwarf2 info, it said there were
> > 120k. 
> > So something isn't adding up here. 
> 
> How do you eliminate them? The vast majority of types in debug info
> are duplicate header file types coming from different source files.
> Reducing this should cut the number much more drastically than
> from 120k to 80k. I'd say by factor of 8 at the very least. 

Easy.
I have a simple hash table, 4096 entries right now.
before we read a type die, we see if the name of the type is in the hash
table (C++ ODR lets me, nobody with a C program has complained they have
the same type in multiple translation units that isn't really the same
type. I suspect it would break other stuff anyway), if it is, we use the
type in the hash table.
Otherwise, we enter it into the hash table.
If it's in the hash table, but the name doesn't match, i have some code
(not in gdb 5.0, because it's
noisy and only for profiling, obviously) to tell me it's a collision.
No collisions found.

Theoretically, this should remove all duplicate type info being read in if
there aren't any collisions.

So something else is making them, most likely lookup_pointer_type or
thereabouts.

I guess part of the problem is that we can't free types.

But i still think we generate way too many types.

> 
> That would mean around one type name per 400 lines of code. Since almost all
> type declarations are in header files which are about 1/10 of all sources
> in size, it looks reasonable. 
> 
> -- 
> Anatoly Vorobey,
> mellon@pobox.com http://pobox.com/~mellon/
> "Angels can fly because they take themselves lightly" - G.K.Chesterton
> 

Re: Proposal: convert function definitions to prototyped form

[Kevin Buettner]

On Jun 13, 11:09am, Andrew Cagney wrote:

> Kevin Buettner wrote:
> > 
> > As many of you know, I'm in the midst of purging the use of ``PARAMS''
> > in prototyped function declarations from the gdb sources.  After this
> > activity is concluded, I'd like to begin converting function
> > definitions whose parameters are declared using the traditional C
> > style to the ISO C prototyped form.  I.e, I'd like to convert
> > functions of the form
> 
> Kevin,
> 
> Would a fair summary of this be that you've encountered the following
> road blocks:
> 
> 	o	indent.pro
> 
> 		So that indent gives better results

I don't think this is a serious road block.  While it would be nice to
be able to specify an indent.pro file (e.g., "indent --options-file
gdbtypes.pro"), it's not absolutely necessary for the task at hand.

As I noted in my reply to Eric, I already know the complete list of
types which indent has to know about in order to complete the
protoization task.  It is easy to put this list in the script which
does the conversion.  I.e,

	@typelist = qw(ADDR32 B_TYPE ... value_ptr xdrproc_t);
	$indentoptions = '-T ' . join(' -T ', @typelist);

> 	o	a drain of some of the backlog of
> 		patches (pascal I believe is now
> 		going in).
> 
> 		Apple and HP?

I think the patch backlog is the real road block.

However, even for this road block, the only reason for delaying the
protoization activity is to make it easier on the people doing the
patch integration.  Perhaps setting a date for the protoization
activity would help motivate the patch integrators to clear some of
the backlog?

> No one has objected to the principal (well not on gdb-patches :-) and
> the tool would be based on perl rather than the the gcc thing.
> 
> Once those blockages are cleared, it can be scheduled and can go
> through?

I still have a little over a week to go on the PARAMS elimination
activity, so any time after then is good for me.  How does midnight
GMT of Sunday July 9 sound?  That's about four weeks away.  Is that
enough time for the patch integrators to clear the patch backlog?

Re: Proposal: convert function definitions to prototyped form

[Eli Zaretskii]

> Date: Mon, 12 Jun 2000 16:30:44 -0700
> From: Eric Bachalo <ebachalo@redhat.com>
>
> Not sure how ported etags and python are for all the hosts that
> compile GDB though.

What platforms cause your concern?

Re: Proposal: convert function definitions to prototyped form

[Andrew Cagney]

Kevin Buettner wrote:
> 
> On Jun 3,  6:58am, Eli Zaretskii wrote:
> 
> > So with a script, we will always need a verification tool that can be
> > trusted to find any potential bugs introduced by reformatting.
> 
> Right.
> 
> That's why I wrote check-decls (see
>     http://sourceware.cygnus.com/ml/gdb/2000-06/msg00028.html
> ) which takes the result of comparing (via diff -u) the original
> sources with the protoized sources and produces a C source file in
> which the portions from the protoized sources are used to construct
> prototypes and the portions from the original sources are used to
> construct (potentially?) corresponding function definitions.  We can
> then invoke the C compiler (gcc -c -Wall) on the result and see what
> kinds of warnings and errors are produced.

Rather than -Wall (badly missnamed) I'd suggest several more carefully
selected flags such as: -Wimplict -Wreturn-type -Wstrict-prototypes
-Wmissing-declarations.  No need to make life difficult for your self
:-)

	Andrew

Re: Proposal: convert function definitions to prototyped form

[Kevin Buettner]

On Jun 3,  2:37pm, Eli Zaretskii wrote:

> > Date: Sat, 3 Jun 2000 10:50:39 -0700
> > From: Kevin Buettner <kevinb@cygnus.com>
> >
> > That's why I wrote check-decls (see 
> >     http://sourceware.cygnus.com/ml/gdb/2000-06/msg00028.html
> > ) which takes the result of comparing (via diff -u) the original
> > sources with the protoized sources and produces a C source file in
> > which the portions from the protoized sources are used to construct
> > prototypes and the portions from the original sources are used to
> > construct (potentially?) corresponding function definitions.  We can
> > then invoke the C compiler (gcc -c -Wall) on the result and see what
> > kinds of warnings and errors are produced.
> 
> I saw that script, but I don't like the idea to depend on a human to
> judge what GCC warnings are okay to ignore and what aren't.  I'd
> prefer an automated tool that would give a definite yes/no answer, if
> that's possible.

I see your point.

If I do as Andrew suggests and prevent fix-decls from looking at the
*-share directories, that will prevent the arg_type conflict which
is responsible for the errors.  That leaves only the exit() warning
from standalone.c to worry about.  I can probably devise a solution
which will prevent that warning from occurring as well.

Anyway... I will see if I can arrange it so that any error or warning
indicates a problem and no errors or warnings indicates that all's
well.

Kevin

Re: Proposal: convert function definitions to prototyped form

[Eli Zaretskii]

> Date: Sat, 3 Jun 2000 10:50:39 -0700
> From: Kevin Buettner <kevinb@cygnus.com>
>
> That's why I wrote check-decls (see 
>     http://sourceware.cygnus.com/ml/gdb/2000-06/msg00028.html
> ) which takes the result of comparing (via diff -u) the original
> sources with the protoized sources and produces a C source file in
> which the portions from the protoized sources are used to construct
> prototypes and the portions from the original sources are used to
> construct (potentially?) corresponding function definitions.  We can
> then invoke the C compiler (gcc -c -Wall) on the result and see what
> kinds of warnings and errors are produced.

I saw that script, but I don't like the idea to depend on a human to
judge what GCC warnings are okay to ignore and what aren't.  I'd
prefer an automated tool that would give a definite yes/no answer, if
that's possible.

Re: Proposal: convert function definitions to prototyped form

[Kevin Buettner]

On Jun 3,  6:58am, Eli Zaretskii wrote:

> So with a script, we will always need a verification tool that can be
> trusted to find any potential bugs introduced by reformatting.

Right.

That's why I wrote check-decls (see 
    http://sourceware.cygnus.com/ml/gdb/2000-06/msg00028.html
) which takes the result of comparing (via diff -u) the original
sources with the protoized sources and produces a C source file in
which the portions from the protoized sources are used to construct
prototypes and the portions from the original sources are used to
construct (potentially?) corresponding function definitions.  We can
then invoke the C compiler (gcc -c -Wall) on the result and see what
kinds of warnings and errors are produced.

E.g, in an earlier (than the one I posted) version of fix-decls,
I hadn't yet handled comma separated parameter lists and so the
following:

    foo (a, b, c)
         int a;
         char *b, *c;

was getting transformed into

    foo (int a, int b, char *b, *c)

This type of mistake would've been quickly caught by check-decls +
gcc.  (As it was, I caught it myself because I was looking for it.)

Also, since my fix-decls script merely looks for patterns which
appear to be function definitions, it was finding

if (overload_debug)
{

in find_overload_match() in valops.c and turning this into

if (int overload_debug)
{

(Note that the ``if'' is at the beginning of the line in this function.)

I found this one when I did a test build, but check-decls + the C
compiler would have caught this one too.  (fix-decls was quickly
changed so that it no longer gets tripped up by this code.)  Also, note
that check-decls would've caught this mistake even if the the
construct in question had appeared in some #if 0'd code whereas doing
a build wouldn't have.

I think it could still happen that something might slip by that won't
work in the real code, but now that I've written check-decls, I think
it is much, much less likely.

Kevin

Re: Proposal: convert function definitions to prototyped form

[Eli Zaretskii]

> Date: Fri, 2 Jun 2000 12:10:42 -0700
> From: Kevin Buettner <kevinb@cygnus.com>
>
> > 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?

I think the real problem is not the number of developers, but the
number of different configurations, and also different data types and
functions concealed behind macros.  `protoize' needs everything to be
explicit, so it is not easy to run it on multi-platform project that
uses macros to hide system dependencies (since you want the same
macros back in the reformatted sources).

This is a disadvantage of `protoize'.  Its significant advantage is
that its output is *always* correct, because it takes the info ``from
the horse's mouth'': the compiler itself.  In contrast, a script is
simply a text-processing tool: it really doesn't understand the
semantics of the source.  In fact, it doesn't really understand the
syntax very well.

So with a script, we will always need a verification tool that can be
trusted to find any potential bugs introduced by reformatting.

> > 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.)

Comparing object files generally doesn't work.  COFF (at least the
variety used by DJGPP) is another case: if I compile the same source
twice in a row, I get different object files (IIRC, the time stamp is
recorded inside).

One method I can suggest is to compile the source without
optimizations, then run "objdump --disassemble" and compare the output
of `objdump' for the two source files (original and reformatted).  (I
suggest to disable optimizations because it's possible that ANSI
source allows the compiler to produce more optimal code that the K&R
source.)

Note that, since you need to compile the source to make sure
reformatting didn't screw up anything, you essentially get the same
problems you had with `protoize', albeit through the back door: you
need to build for all supported configurations to make sure nothing's
broken.

Re: Proposal: convert function definitions to prototyped form

[David Taylor]

    Date: Fri, 2 Jun 2000 12:10:42 -0700
    From: Kevin Buettner <kevinb@cygnus.com>

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

    >       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

Sorry, I didn't consider it relevant to what I was replying to.

    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.

I don't consider the setup issue to be a big issue.  It requires some
time, but it doesn't require much human time.  Configure in the source
tree, then build.  Setup done.

During the commit phase, you don't commit anything that is new -- you
only commit files that existed prior to doing the configure.  No
separate tree needed.

    >       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.

Since Stan previously ran indent on all of the files, re-running it
*should* produce no change for stuff that hasn't changed.  Should
produce no change.  I'm not saying it won't.  If it changes its own
output when presented with it as input, I would consider that a bug.

    > 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.)

It was at a former company.  There were a small number of active
developers all within the company.  The total project was around
200-250K lines of code.  There was a great deal less use of #if within
the sources than there is in GDB...

[...]
    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.

I agree that it should be a separate activity.

I think that your script is a good thing.  And due to the amount of
hand editing that appears to be necessary with protoize vs the amount
that appears to be necessary with your script, I think we should use
your script.

Ultimately, though, since you're doing the work, you get to select the
tool.  So long as it is fairly reliable and reasonably quick (quick in
terms of *your time*, not in terms of *elapsed time*), it doesn't
ultimately matter to me what tool you use.  (Which is not to say that
I am not interested in knowing what tool you use -- I am interested.)

Like I said, I think your script is a good thing.  And I look forward
to you using it to protoize the bulk of the GDB sources.

    Kevin

David

Re: Proposal: convert function definitions to prototyped form

[Kevin Buettner]

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

Re: Proposal: convert function definitions to prototyped form

[David Taylor]

    Date: Fri, 2 Jun 2000 00:50:19 -0700
    From: Kevin Buettner <kevinb@cygnus.com>
[...]
	2) ``protoize'' fails to convert functions disabled by ifdefs for
	   the given platform.  OTOH, on some other platform(s), these
	   functions might not be disabled and would be converted.  E.g,
	   in breakpoint.c, on my GNU/Linux/x86 machine, protoize failed
	   to convert create_longjmp_breakpoint() which is protected by an
	   "#ifdef GET_LONGJMP_TARGET".

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.

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

	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.

	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!

    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.

    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".

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

    errors so that you can build again.  OTOH, the script may leave
    certain portions of the file alone that could possibly have be
    converted had the script been a little smarter.  The things that the
    script fails to convert *will* have to be fixed later on (in order to
    complete the cleanup activity), either by another script, or by hand. 
    For the PARAMS purging activity, I have spent a fair amount of time
    examining the diffs to make sure that this is indeed the case.  (And
    I intend to do the same for the activity in this proposal.)

Good.

    The reason that it is so important to avoid any hand edits is that we
    want people who have local repositories or checked out source trees to
    be able to run these conversion scripts against them so that merges
    will be less painful.  (Even easy.)

Agreed.

    With that said, keeping in mind the problems I noted above, I conclude
    that ``protoize'' is not the appropriate tool for us to use to convert
    function definitions in the gdb sources to their prototyped form.

I only consider 3) -- screwing up on PTR types -- to be serious; the
others seem minor enough.
[...]

    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...

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.