On 06/28 07:24:08, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> 
> 
> > -----Original Message-----
> > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Joyce
> > Kong
> > Sent: Wednesday, June 28, 2017 5:14 AM
> > To: lng-odp@lists.linaro.org
> > Cc: Joyce Kong <joyce.k...@arm.com>
> > Subject: [lng-odp] [PATCHv2] linux-gen: scheduler: modular scheduler
> > interface
> > 
> > The modular scheduler interface in odp_schedule_if.h includes functions
> > from pktio and queue. It needs to be cleaned out. The pktio/queue related
> > functions should be moved to pktio/queue internal header file.
> 
> Sched_cb_xxx() functions are the interface from a scheduler towards other 
> parts of the system. So, those calls are not in the scheduler  interface file 
> by mistake.

Generally speaking, function declarations should exist in the .h
file of the .c file that contains the definitions. These functions
are defined in queue.c and called from schedule.c. The declarations
should be in queue.h. It can be as simple as that.

And, usually 'cb' or 'callback' is used in naming a function that is
called through a function pointer. So, the naming is off here as well
since these functions are always called via a direct function call.

If you have to caution the user as to why something is written the
way it is, and that it is not a mistake, it better be a juicy topic.
Following best practices such as placing function declarations in
the .h file of the .c file that they are defined in is not a juicy
topic.

> If we now agree that queues and scheduler come as a package, tighter 
> integration between queue.c and 3 original schedulers could be done. But 
> preferably through another header file than odp_queue_internal.h, since that 
> exposes internals of the queue block.
> 
> This should *not* be done for pktio. It's the sched -> pktio interface as of 
> today.
> 
> I can look into this, since I'd inline as much as possible of those 
> sched_cb_queue_xxx() functions anyway. Also this work should not affect the 
> ARM scheduler, right?
> 
> -Petri
> 
>  
> 

Reply via email to