06/10/2023 10:46, David Marchand: > On Thu, Oct 5, 2023 at 12:09 PM Mattias Rönnblom <hof...@lysator.liu.se> > 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.