Thank you for your answer!

I send you a new patch
I have corrected the errors you raised.

I have make my patch compatible with the old use of c_register_pragma 
and c_register_pragma_with_expansion.

I don't know what is the best solution, maybe changing every call 
c_register_pragma allows to keep a more clear code. I can do it, if you 
think it is better.

I have successfully compiled gcc with my patch and I have tried it with 
a modified version of gcc/testsuite/g++.dg/plugin/pragma_plugin.c.

Pierre Vittet

On 02/06/2011 19:51, Tom Tromey wrote:
>>>>>> "Pierre" == Pierre<p.vittet@laposte.net>  writes:
>
> Pierre>  I have changed this handler in order to accept a second parameter
> Pierre>  which is a void *, allowing to give extra datas to the handler. I
> Pierre>  think this data field might be of general use: we can have condition
> Pierre>  or data at register time that we want to express in the handler. I
> Pierre>  guess this is a common way to pass data to an handler function.
>
> I can't approve or reject this patch, but the idea seems reasonable
> enough to me.
>
> Pierre>  I would like your opinion on this patch! Thanks!
>
> It has a number of formatting issues.
>
> Pierre>  +typedef void (*pragma_handler)(struct cpp_reader *, void * );
>
> No space after the final "*".
>
> Pierre>  +/* Internally use to keep the data of the handler.  */
> Pierre>  +struct internal_pragma_handler_d{
>
> Space before the "{".
>
> Pierre>  +  pragma_handler handler;
> Pierre>  +  void * data;
>
> No space.  Lots of instances of this.
>
> Pierre>   /* A vector of registered pragma callbacks.  */
> Pierre>  +/*This is never freed as we need it during the whole execution */
>
> Coalesce the two comments.  The comment formatting is wrong, see GNU
> standards.
>
> Pierre>         ns_name.space = space;
> Pierre>         ns_name.name = name;
> Pierre>  +
> Pierre>         VEC_safe_push (pragma_ns_name, heap, registered_pp_pragmas,&ns_name);
>
> Gratuitous newline addition.
>
> Pierre>  +      ihandler->handler = handler;
> Pierre>  +      ihandler->data = data;
>
> I didn't see anything that initialized ihandler.
>
> Pierre>  +      VEC_safe_push (internal_pragma_handler, heap, registered_pragmas,
> Pierre>  +&ihandler);
>
> I think you wanted just `internal_pragma_handler ihandler', no "*", for
> the definition.
>
> Pierre>  +c_register_pragma (const char *space, const char *name, pragma_handler handler,
> Pierre>  +                   void * data)
>
> There are lots of calls to this that you did not update.
> Do a recursive grep to see.
>
> One way to avoid a massive change is to add a new "overload" that passes
> in the data to c_register_pragma_1; and then change the "legacy"
> functions to pass NULL.
>
> I don't know if that approach is ok (it is typical in gdb...), so if
> not, you have to update all callers.
>
> Tom
>