diff mbox

Ping! gengtype plugin improvement last2round - patch 1 [declprog]

Message ID 20101017102619.b33e5b70.basile@starynkevitch.net
State New
Headers show

Commit Message

Basile Starynkevitch Oct. 17, 2010, 8:26 a.m. UTC 
On Thu, 14 Oct 2010 15:58:55 +0200
Basile Starynkevitch <basile@starynkevitch.net> wrote:
> 
> Committed as revision 165470 to the trunk, with the requested space changes.

There remains a bug in the code,  plugin_files was not allocated.

The patch below could fix it, but it belongs to stage 2. For me the
most important is to get all ours (Jeremie Salvucci's & mine, Basile's)
patches reviewed before end of stage 1. So if a reviewer is tight on
time (and Diego surely is), please give priority to
http://gcc.gnu.org/ml/gcc-patches/2010-10/msg01262.html & Laurynas
space improvement from
http://gcc.gnu.org/ml/gcc-patches/2010-10/msg01300.html no to this
patch.

################### patch to trunk 165574
################### gcc/ChangeLog entry
2010-10-17  Basile Starynkevitch  <basile@starynkevitch.net>
	* gengtype.c (parse_program_options): Allocate plugin_files.
###################

But again, I know that the rarest resource is reviewer's time. To
reviewers, please review
http://gcc.gnu.org/ml/gcc-patches/2010-10/msg01262.html before this
patch.

I was almost tempted to commit this fix to trunk as obvious, but I did
not dare do that. There might also be perhaps in some comment a missing
space (I forgot where).

Again, for reviewers, please review
http://gcc.gnu.org/ml/gcc-patches/2010-10/msg01262.html before this
patch.

On a more general note, the set of gengtype patches I am trying to push
to trunk don't change the generated gt*.[ch] files for the trunk, but I
did not test it a lot for plugins. Correcting the plugin gengtype
behavior is for stage 2, and I need to have all my patches accepted
(including the entire gengtype-state.c file of [wstate] patch chunk)
before having gengtype working for plugin. And gengtype never really
worked for plugins anyway (because of the current requirement of
needing build & source tree of gcc, which this series of patch should
fix).

I hope that stage 1 will end after the end of the GCC Summit, otherwise
I am desperate about getting the gengtype plugin improvements into 4.6
(since reviewers don't have time to review it).

I did commit this obvious patch to MELT branch.

Cheers.

Comments

Laurynas Biveinis Oct. 18, 2010, 3:47 a.m. UTC | #1 
2010/10/17 Basile Starynkevitch <basile@starynkevitch.net>:
> There remains a bug in the code,  plugin_files was not allocated.

> 2010-10-17  Basile Starynkevitch  <basile@starynkevitch.net>
>        * gengtype.c (parse_program_options): Allocate plugin_files.

The patch is OK - and it is obvious, IMHO.

However, while reviewing this I found the following in main:

      if (nb_plugin_files <= 0 || !plugin_files)
        fatal ("No plugin files given in plugin mode for %s",
               plugin_output_filename);

nb_plugin_files is of size_t, which is unsigned. Please replace with ==.

Thanks,
diff mbox

Patch

Index: gcc/gengtype.c
===================================================================
--- gcc/gengtype.c	(revision 165574)
+++ gcc/gengtype.c	(working copy)
@@ -4395,6 +4395,7 @@  parse_program_options (int argc, char **argv)
       if (optind >= argc)
 	fatal ("no source files given in plugin mode");
       nb_plugin_files = argc - optind;
+      plugin_files = XNEWVEC (char*, nb_plugin_files);
       for (i = 0; i < (int) nb_plugin_files; i++)
 	{
 	  char *name = argv[i + optind];