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

From: Keller, Jacob E <jacob.e.keller@intel.com>
Date: Jul 16, 2014, 19:50 UTC
Message-ID: <1405540201.15392.3.camel@jekeller-desk1.amr.corp.intel.com>
In-Reply-To: <CAPc5daUhmeucDOakXpR_ViS0cfFDEyXUD_xoeP6qeCg33dW+1A@mail.gmail.com>

There was no way to get the current error routine now, and I figured that a stack was a simple way of saving the old routine.

Essentially these two paths would be the same as a "save/restore" except we manage it via a stack. I don't really see how that would end up any different. I mean I don't mind either approach.

Thanks, Jake

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

Show 34 quoted lines

> I actually am not a big fan of "stack" for a thing like this, to be honest.
> Wouldn't it be sufficient for the callers who want specific behaviour from
> its callees to
> 
>  - save away the current error/warning routines;
>  - set error/warning routines to its own custom versions;
>  - call the callees;
>  - set error/warning routines back to their original; and
>  - return from it
> 
> at least in the code paths under discussion?
> 
> 
> On Tue, Jul 15, 2014 at 4:26 PM, Keller, Jacob E
> <jacob.e.keller@intel.com> wrote:
> > On Tue, 2014-07-15 at 15:47 -0700, Junio C Hamano wrote:
> >> 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.
> >>
> >
> > Also curious what your thoughts on making every set_*_routine to be a
> > stack? For now I was only planning on error but it maybe makes sense to
> > change them all?
> >
> > Thanks,
> > Jake
> >