Re: [PATCH v8 1/4] usage: make error functions a stack

From: Keller, Jacob E <jacob.e.keller@intel.com>
Date: Jul 15, 2014, 23:24 UTC
Message-ID: <1405466672.2577.14.camel@jekeller-desk1.amr.corp.intel.com>
In-Reply-To: <xmqq7g3etf1o.fsf@gitster.dls.corp.google.com>

On Tue, 2014-07-15 at 15:47 -0700, Junio C Hamano wrote:

Show 10 quoted lines

> Jacob Keller <jacob.e.keller@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.

Show 20 quoted lines

> > +/* 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