Reviewers of https://gerrit.iotivity.org/gerrit/#/c/8603/ rightly pointed
out that it is not maximally portable. FWIW I do not think patches that
make Iotivity run on a new platform without breaking existing platforms
should be asked to also solve problems on all possible platforms, but
that's a separate issue. The purpose of this message is to sketch a
possible way of dealing with timers in a portable way.
Currently the following files contain calls to clock_gettime, some also use
gettimeofday; I'm not sure if other time functions are used.
ok all current time: resource/c_common/ocrandom/src/ocrandom.c
ok all current time: resource/c_common/oic_time/src/oic_time.c
ok android current time:
resource/csdk/connectivity/common/src/camutex_pthreads.c
ok TIMEOUT (BLE only):
resource/csdk/connectivity/src/bt_le_adapter/linux/caleinterface.c
ok current time: resource/csdk/connectivity/src/caretransmission.c
ok current time: resource/csdk/connectivity/test/camutex_tests.cpp
ok current time: resource/csdk/logger/src/logger.c
ok current time: resource/csdk/routing/src/routingtablemanager.c
ok android only? resource/csdk/security/src/directpairing.c
x TIMEOUT: resource/csdk/security/provisioning/src/pmutility.c
x TIMEOUT: plugins/zigbee_wrapper/telegesis_wrapper/src/telegesis_socket.c
mimic oic_time.c
x TIMEOUT:
service/easy-setup/mediator/richsdk/src/RemoteEnrolleeResource.cpp
Here "ok" means the code compiles on Darwin, x means a patch is required
for Darwin. "Current time" and "TIMEOUT" refer to what the code does with
time.
Code that runs on Darwin uses #ifdef on _POSIX_TIMERS to select
"clock_gettime" else "gettimeofday" ; Darwin has the latter not the former.
In patch 8603 (link above) I tried to make the code in the offending files
follow the working code in the other files.
The reviewers pointed out that:
* The Windows port adopts an autoconf-like approach using e.g. "#if
defined(HAVE_GETSYSTEMTIMEASFILETIME)" to select Windows code.
* There are issues with _POSIX_TIMERS, CLOCK_MONOTONIC, etc. I haven't been
able to find an unambiguous definition of exactly what _POSIX_TIMERS
implies. See POSIX Conformance
<http://pubs.opengroup.org/onlinepubs/007904875/basedefs/xbd_chap02.html>,
Feature
Groups <http://pubs.opengroup.org/onlinepubs/007908775/xsh/feature.html>etc.
Example: OpenBSD only partially supports the posix "timer option" so it
does not define _POSIX_TIMERS but it does have clock_gettime,
CLOCK_MONOTONIC, and CLOCK_REALTIME (manpage
<http://man.openbsd.org/clock_gettime.2>).
* There are lots of other OSes, e.g. the BSDs (Free/Net/etc.) and many
RTOSes, all of which may do things differently.
There are (at least) two ways to get a portable implementation: conditional
compilation *within* files, controlled by #ifdef; and conditional
compilation *of* files, controlled at the SConscript level.
In the case of clock_gettime etc. I think #ifdef'ed code quickly becomes
unreadable. Take a look at the PMTimeout function in
resource/csdk/security/provisioning/src/pmutility.c, and imagine what it
would look like if we were to add #ifdefs to check for Darwin, OpenBSD,
FreeRTOS, etc. IMO it would become completely unreadable.
An alternative is to split the code into platform-specific c files, and
select the file to use at build time based on feature detection. Taking
pmutililty.c as an example, we might have a "timer" subdir within
provisioning/src, and then for e.g. Darwin we would have timer/darwin.c
containing something like:
...
#include <sys/time.h>
...
OCStackResult PMTimeout(unsigned short waittime, bool waitForStackResponse)
{
OCStackResult res = OC_STACK_OK;
static const struct timeval zeroTime = { .tv_sec = 0, .tv_usec = 0 };
struct timeval startTime = { .tv_sec = 0, .tv_usec = 0 };
struct timeval currTime = { .tv_sec = 0, .tv_usec = 0 };
clock_res = gettimeofday(&startTime, NULL); //FIXME: use
mach_absolute_time?
if (0 != clock_res)
{
return OC_STACK_ERROR;
}
OCStackResult res = OC_STACK_OK;
while (OC_STACK_OK == res)
{
currTime = zeroTime;
clock_res = gettimeofday(&currTime, NULL);
if (0 != clock_res)
{
return OC_STACK_TIMEOUT;
}
long elapsed = (currTime.tv_sec - startTime.tv_sec);
if (elapsed > waittime)
{
return OC_STACK_OK;
}
if (waitForStackResponse)
{
res = OCProcess();
}
}
return res;
}
And similar for the other platforms. This shows that the function is
actually pretty simple so it should be relatively easy to ensure that the
functionality is the same across platforms. Easier than it would be if
everything was stuffed into a giant #ifdef mess, IMO.
Instead of timer/linux.c, we would have timer/posix.c, which would not be
linux-specific (would it work for Tizen?). So to get started we would have:
resource/csdk/security/provisioning/src/timer
- darwin.c
- posix.c
- win32.c
Comments?
Gregg
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.iotivity.org/pipermail/iotivity-dev/attachments/20160615/74dc9073/attachment.html>
