SOCI / soci

Official repository of the SOCI - The C++ Database Access Library
http://soci.sourceforge.net/
Boost Software License 1.0
1.37k stars 472 forks source link

timegm implementation has been added to build on SunOs and AIX #1080

Closed avpalienko closed 9 months ago

avpalienko commented 9 months ago

timegm function is missing on some platforms, e.g. Sun and AIX. We have to check the existence and use self-made version if not

avpalienko commented 9 months ago

Thanks! Is selfmade_timegm() your own code? A quick search finds some other versions of this function which seem a bit more clear and don't use macros, e.g. this one.

Especially if it's your own code, it would be really nice to have at least some trivial unit tests for this function to make sure it works correctly now (which it probably does) and keeps working in the future (as it would be easy to break it with the automated tests only running under platforms not using this function at all).

Thank you for review!

No. It's a copy from internet. Your example does not normalize the argument but timegm does. I've added the unit test and removed the macros

vadz commented 9 months ago

It's a copy from internet.

It would be nice to point out its provenance, if only to have a confirmation that it's available under a compatible licence.

avpalienko commented 9 months ago

It's a copy from internet.

It would be nice to point out its provenance, if only to have a confirmation that it's available under a compatible licence.

Sorry, but I can't find the link, it's been a while

vadz commented 9 months ago

Ugh, sorry, a quick code search finds this code in mktime.c file with

/*
  Copyright (c) 1987-1997, Microsoft Corporation. All rights reserved.
*/

in it so we really can't include it into SOCI, unfortunately.

Could you please get the version from SO (which is implicitly in public domain AFAIK) or find some other implementation under acceptable licence?

avpalienko commented 9 months ago

Ups! :-( Thank you! I've changed implementation to as SO

vadz commented 9 months ago

Merged now, thanks for the update!

Begasus commented 2 months ago

Thanks for this, was hitting this when checking another project that uses SOCI, fixes the build for Haiku. +1