On Tue, 2014-07-15 at 15:47 -0700, Junio C Hamano wrote:
> Jacob Keller <jacob.e.kel...@intel.com> writes:
> 
> >  extern void set_error_routine(void (*routine)(const char *err, va_list 
> > params));
> > +extern void pop_error_routine(void);
> 
> pop that undoes set smells somewhat weird.  Perhaps we should rename
> set to push?  That would allow us catch possible topics that add new
> calls to set_error_routine() as well by forcing the system not to
> link when they are merged without necessary fixes.
> 

I thought about changing set too, but wasn't sure that made sense..?
That does make more sense now though. There *are* valid use cases where
a set_error_routine is used without a pop, (the one current use, I
think).

I'll update this patch with that change.

> > +/* push error routine onto the error function stack */
> >  void set_error_routine(void (*routine)(const char *err, va_list params))
> >  {
> > -   error_routine = routine;
> > +   struct error_func_list *efl = xmalloc(sizeof(*efl));
> > +   efl->func = routine;
> > +   efl->next = error_funcs;
> > +   error_funcs = efl;
> > +}
> > +
> > +/* pop a single error routine off of the error function stack, thus 
> > reverting
> > + * to previous error. Should always be paired with a set_error_routine */
> > +void pop_error_routine(void)
> > +{
> > +   assert(error_funcs != &default_error_func);
> > +
> > +   struct error_func_list *efl = error_funcs;
> 
> decl-after-stmt.  Can be fixed easily by flipping the above two
> lines.

Oh, right yes. I'll fix that in a resend as well.

Thanks,
Jake


Reply via email to