On 2017-01-12 15:34, Kristaps Dzonsons wrote:
Looks good, except for one thing:
The CAVEATS are important because calling gmtime et al from a library might not be expected. (They touch the zoneinfo files, which may not be available on embedded or sandboxed systems.)
Why would that be unexpected? They are standard C library functions. I see no reason not to use them (and there are basically no easy alternatives). If their implementation is broken on some system, then that's a libc problem. So if you ask me, this caveats section is not necessary and only adds confusion.
It's not that libc is broken w/r/t these functions, but it should be noted that they're being used to specify how localtime is being computed.
(Why? First, because according to POSIX, localtime_r might invoke tzset() beforehand, or it might not. Moreover, in chroots or in capabilities environments, localtime might behave like gmtime due to file-system restrictions.)
Perhaps this should be in an IMPLEMENTATION NOTES section instead of the CAVEATS section, however, because it's not a bad thing --- just clarification.
Yes, I think that's better. With the caveats section and the current description ("might interact with the file-system in unexpected ways"), it sounds as if there is a problem. And that's not really true.
I also suggest to rephrase the paragraph in a more informative way. For example something like this:
"The implementation of the dc_datetime_gmtime function may be based on the standard C library time functions, like gmtime(3) or gmtime_r(3)."
BTW, the main reason why those functions are exposed in the first place is to give an application access to exactly the same functions as used by libdivecomputer internally. For simple C/C++ applications, which are using the C library functions too, that doesn't matter. But applications using some kind of framework (Gtk, Qt, .NET, python, etc) usually have their own date/time functions, which may use a completely different implementation (especially on Windows).
(For safety, it should also be noted in dc_parser_get_datetime that the dc_datetime_localtime function might be invoked.)
Isn't that obvious? A function to parse date/time will probably use the library's date/time functions :-)
I forgot to edit Makefile.am in my patch. If you'd like, I can rephrase the CAVEAT as an IMPLEMENTATION NOTE and amend the Makefile.am in another patch?
I did already add the missing Makefile.am bits in my tree. So just the manpages changes.
Jef