On Mon, Feb 3, 2020 at 2:44 PM Gaetan Rivet <gr...@u256.net> wrote:
>
> On 02/02/2020 12:21, Jerin Jacob wrote:
> > On Sun, Feb 2, 2020 at 4:08 PM Stephen Hemminger
> > <step...@networkplumber.org> wrote:
> >>
> >> On Fri, 31 Jan 2020 22:31:57 +0530
> >> <jer...@marvell.com> wrote:
> >>
> >>> +
> >>> +#define set_err(err, where, fmt, ...) do {                               
> >>>    \
> >>> +     graph_err(fmt, ##__VA_ARGS__);                                      
> >>>    \
> >>> +     rte_errno = err;                                                    
> >>>    \
> >>> +     goto where;                                                         
> >>>    \
> >>> +} while (0)
> >>
> >> I dislike this macro, it makes static analysis harder and requires
> >> the reader to know that the argument is a goto target. And since it is 
> >> lower
> >> case, implies that it is a function. Usually macros are in upper case.
> >>
> >> It makes the code smaller but a cost of being different which impacts the
> >> readability of the code.
> >
> > I don't like macro either. That's the only case where I have used
> > macro, Even in the fast path,
> > I did multiple rework to remove macro. Without that macro, that code bloats
> > and will have a lot repeatable code. I have a preference in using _goto_ to 
> > have
> > a unified exit to simply the stuff and therefor maintain the code.
> > You could see the amount of verification done in rte_graph_create(). So, 
> > IMO,
> > In this case, it is justified to use the macro.
> >
>
> Hi Jerin,

Hi Gaetan,

>
> Renaming it in uppercase and making clear there will be a jump could 
> alleviate some concerns?
> Something like SET_ERR_JMP() maybe or SET_ERR_GOTO().

OK. I will change to SET_ERR_JMP() in the next version.

Reply via email to