06/10/2023 10:46, David Marchand:
> On Thu, Oct 5, 2023 at 12:09 PM Mattias Rönnblom <[email protected]>
> wrote:
> > >> +static int
> > >> +evd_lookup_handler_idx(struct rte_dispatcher_lcore *lcore,
> > >> + const struct rte_event *event)
> > >
> > > Wrt DPDK coding tyle, indent is a single tab.
> > > Adding an extra tab is recommended when continuing control statements
> > > like if()/for()/..
> > >
> >
> > Sure, but I don't understand why you mention this.
>
> I wanted to remind the DPDK coding style which I try to more closely
> enforce for new code.
> indent is off in this file (especially for function prototypes with
> multiple tabs used).
>
> >
> > > On the other hand, max accepted length for a line is 100 columns.
> > >
> > > Wdyt of a single line for this specific case?
> >
> >
> > Are you asking why the evd_lookup_handler_idx() function prototype is
> > not a single line?
> >
> > It would make it long, that's why. Even if 100 wide lines are allowed,
> > it doesn't means the author is forced to use such long lines?
>
> I find it more readable.
> If you want to stick to 80 columns, please comply with a single tab for
> indent.
I think this is a case of continuation line, so it should be 2 tabs.
We can make it clear in the doc.
> > >> +static int
> > >> +evd_set_service_runstate(struct rte_dispatcher *dispatcher, int state)
> > >> +{
> > >> + int rc;
> > >> +
> > >> + rc = rte_service_component_runstate_set(dispatcher->service_id,
> > >> + state);
> > >> +
> > >> + if (rc != 0) {
> > >> + RTE_EDEV_LOG_ERR("Unexpected error %d occurred while
> > >> setting "
> > >> + "service component run state to %d\n",
> > >> rc,
> > >> + state);
> > >> + RTE_ASSERT(0);
> > >
> > > Why not propagating the error to callers?
> > >
> > >
> >
> > The root cause would be a programming error, hence an assertion is more
> > appropriate way to deal with the situation.
>
> Without building RTE_ENABLE_ASSERT (disabled by default), the code
> later in this function will still be executed.
Don't forget you are writing a library, so you shouldn't stop runtime
because of an error. It is always better to return errors.
Assert should be used only for debugging, that's why it is disabled by default.
