On 06/07/16 09:50, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: Maxim Uvarov [mailto:[email protected]]
Sent: Monday, June 06, 2016 9:16 PM
To: Savolainen, Petri (Nokia - FI/Espoo) <[email protected]>;
[email protected]
Subject: Re: [lng-odp] [PATCH v2 4/6] linux-gen: sched: add skeleton for
strict priority
On 06/06/16 16:31, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: lng-odp [mailto:[email protected]] On Behalf Of
Maxim
Uvarov
Sent: Monday, June 06, 2016 3:22 PM
To: [email protected]
Subject: Re: [lng-odp] [PATCH v2 4/6] linux-gen: sched: add skeleton
for
strict priority
WARNING: externs should be avoided in .c files
#94: FILE: platform/linux-generic/odp_schedule_if.c:9:
+extern const schedule_fn_t schedule_sp_fn;
WARNING: externs should be avoided in .c files
#95: FILE: platform/linux-generic/odp_schedule_if.c:10:
+extern const schedule_fn_t schedule_default_fn;
odp_schedule_if.c has externs (already in the repo) since this is the
single place that needs to refer those and select between them (#ifdef).
It's cleaner to have both externs and the ifdef co-located, than separated
into a header and the c file. Externs in header would not provide any
benefit here, since there's single references to those.
odp_schedule_if.c
extern const schedule_fn_t schedule_sp_fn;
extern const schedule_api_t schedule_sp_api;
extern const schedule_fn_t schedule_default_fn;
extern const schedule_api_t schedule_default_api;
#ifdef ODP_SCHEDULE_SP
const schedule_fn_t *sched_fn = &schedule_sp_fn;
const schedule_api_t *sched_api = &schedule_sp_api;
#else
const schedule_fn_t *sched_fn = &schedule_default_fn;
const schedule_api_t *sched_api = &schedule_default_api;
#endif
- Petri
Petri, I think we have externs before we started to use checkpatch.pl
and agree that code should be cleared after we do later fixes.
To be consistent here we have to follow previous agreement.
For current case you have references in odp_schedule_sp.c and
odp_schedule_if.c, but both of them do:
#include <odp_schedule_if.h>
so for me it's more logical to have declarations there and remove extern
word at all.
Maxim.
Odp_schedule_if.c was introduced in my last patch series last week. Back then I
documented that chechpatch outputs a warning for the extern. The patch set was
reviewed and integrated. Now this patch renames the extern in repo and adds
another one (for SP scheduler integration).
The point here is clean interface definition (content of odp_schedule_if.h must
not include anything extra). A scheduler implementation includes the interface
file and implements it, but does not have to know anything about other
scheduler implementations (see any extern const schedule_fn_t scheduler_xyz).
The only place in our code base that needs to see all implementations and
select/bind the one that is used (with the #ifdef) is odp_scheduler_if.c.
Checkpatch assumes here that multiple .c files include the same extern
definition, in which case it makes sense to do a single extern definition in a
header. This is not the case here, by definition odp_scheduler_if.c is the
single .c file that should ever see those externs. So, it's better to contain
those in the file itself than spread around in headers (just to make checkpatch
happy).
-Petri
Ok, if nobody cares I apply this and if it will be needed we will clean
up code later.
Maxim.
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
