[flashrom] [PATCH] register_shutdown for execution on programmer shutdown

[Michael Karcher]

Am Samstag, den 13.02.2010, 04:28 +0100 schrieb Carl-Daniel Hailfinger:
> On 13.02.2010 00:59, Michael Karcher wrote:
> > Am Freitag, den 12.02.2010, 03:59 +0100 schrieb Carl-Daniel Hailfinger: 
> >   
> >> +#define SHUTDOWN_MAXFN 4
> >> +static int shutdown_fn_count = 0;
> >> +void (*shutdown_fn[SHUTDOWN_MAXFN]) (void);
> >>     
> > style hint: Use a typedef for the function pointer type:
> > typedef void (*shutdown_proc)(void);
> I thought about using a typedef, but given that most people expect
> typedefs in the form
> typedef type identifier;
> whereas the suggested typedef has the form
> typedef first-part-of-type identifier second-part-of-type;
> a typedef may actually decrease readability for people to don't know
> that such typedefs exist.
I think if reading this form of typedef is difficult (which it is) for
someone, reading the array declaration is even more difficult for that
person.

> About the name of the typedef: I think we should use the name
> atexit_func or something like that to make it obvious we're basically
> copying atexit.
That's a nice idea, you can go for it.

> However, if we use a struct, the typedef issue won't have a noticeable
> impact anymore AFAICS.
It does have: It makes the prototype of register_shutdown more readable.

> struct shutdown_function_data {
> 	void (*func) (void *),
> 	void *data
> } shutdown_fn[4];
> int register_shutdown(void (*func)(void *), void *data);
Adding the void* parameter is a good idea.

> Thanks for the review. I'll wait with a commit until you comment on my
> review response.
If you add the void* parameter, I will Ack the patch soon.

Regards,
  Michael Karcher