[vlc-devel] [PATCH v2 2/4] compat: add clock_nanosleep for darwin

[David Fuhrmann]

> Am 15.03.2020 um 01:49 schrieb Marvin Scholz <epirat07 at gmail.com>:
> 
> ---
> compat/clock_nanosleep.c | 109 +++++++++++++++++++++++++++++++++++++++
> configure.ac             |   4 +-
> include/vlc_fixups.h     |   7 +++
> 3 files changed, 119 insertions(+), 1 deletion(-)
> create mode 100644 compat/clock_nanosleep.c
> 
> diff --git a/compat/clock_nanosleep.c b/compat/clock_nanosleep.c
> new file mode 100644
> index 0000000000..aea702dfa9
> --- /dev/null
> +++ b/compat/clock_nanosleep.c
> @@ -0,0 +1,109 @@
> +/*****************************************************************************
> + * clock_nanosleep.c: POSIX clock_nanosleep() replacement
> + *****************************************************************************
> + * Copyright © 2020 VLC authors and VideoLAN
> + *
> + * Author: Marvin Scholz <epirat07 at gmail dot com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> + *****************************************************************************/
> +
> +#ifdef __APPLE__
> +
> +#ifdef HAVE_CONFIG_H
> +# include <config.h>
> +#endif
> +
> +#include <assert.h>
> +#include <pthread.h>
> +#include <sys/errno.h>
> +#include <sys/types.h>
> +#include <sys/time.h>
> +#include <sys/sysctl.h>
> +#include <mach/clock_types.h>
> +
> +#define timespecs_sub(tsa, tsb, tsr)                                \
> +    do {                                                            \
> +            (tsr)->tv_sec = (tsa)->tv_sec - (tsb)->tv_sec;          \
> +            (tsr)->tv_nsec = (tsa)->tv_nsec - (tsb)->tv_nsec;       \
> +            if ((tsr)->tv_nsec < 0) {                               \
> +                    (tsr)->tv_sec--;                                \
> +                    (tsr)->tv_nsec += NSEC_PER_SEC;                 \
> +            }                                                       \
> +    } while (0)
> +

Hi,

Small comment here: I would really propose to avoid such huge defines. I believe this is really kinda an anti pattern, such thinks should be helper functions instead if needed.

In this case its only used once, so not sure why we need the define at all.

BR. David

> +#define timespec_zero(ts) \
> +    ((ts)->tv_sec == 0 && (ts)->tv_nsec == 0)
> +
> +int clock_nanosleep(clockid_t clock_id, int flags,
> +        const struct timespec *rqtp, struct timespec *rmtp)
> +{
> +    // Validate timespec
> +    if (rqtp == NULL || rqtp->tv_sec < 0 ||
> +            rqtp->tv_nsec < 0 || (unsigned long)rqtp->tv_nsec >= NSEC_PER_SEC) {
> +        errno = EINVAL;
> +        return -1;
> +    }
> +
> +    // Validate clock
> +    switch (clock_id) {
> +        case CLOCK_MONOTONIC:
> +        case CLOCK_REALTIME:
> +            break;
> +        default:
> +            errno = EINVAL;
> +            return -1;
> +    }
> +
> +    if (flags == TIMER_ABSTIME) {
> +        struct timespec ts_rel;
> +        struct timespec ts_now;
> +
> +        do {
> +            // Get current time with requested clock
> +            if (clock_gettime(clock_id, &ts_now) != 0)
> +                return -1;
> +
> +            // Calculate relative timespec
> +            timespecs_sub(rqtp, &ts_now, &ts_rel);
> +
> +            // Check if time already elapsed
> +            if (ts_rel.tv_sec < 0 || timespec_zero(&ts_rel)) {
> +                pthread_testcancel();
> +                return 0;
> +            }
> +
> +            // "The absolute clock_nanosleep() function has no effect on the
> +            // structure referenced by rmtp", so do not pass rmtp here
> +        } while (nanosleep(&ts_rel, NULL) == 0);
> +
> +        // If nanosleep failed or was interrupted by a signal,
> +        // return so the caller can handle it appropriately
> +        return -1;
> +    } else if (flags == 0) {
> +        return nanosleep(rqtp, rmtp);
> +    } else {
> +        // Invalid flags
> +        errno = EINVAL;
> +        return -1;
> +    }
> +
> +    assert(0);
> +    return -1;
> +}
> +
> +#else
> +# error clock_nanosleep not implemented on your platform!
> +#endif
> diff --git a/configure.ac b/configure.ac
> index 134cae50a6..c20ab3c75c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -181,7 +181,9 @@ case "${host_os}" in
> 
>     dnl macOS 10.11 lacks clock_gettime, introduced with 10.12
>     AC_LIBOBJ([clock_gettime])
> -    AC_CHECK_FUNCS([clock_gettime])
> +    dnl macOS lacks clock_nanosleep
> +    AC_LIBOBJ([clock_nanosleep])
> +    AC_CHECK_FUNCS([clock_gettime clock_nanosleep])
> 
>     AC_EGREP_CPP(yes,
>             [#import <TargetConditionals.h>
> diff --git a/include/vlc_fixups.h b/include/vlc_fixups.h
> index 055bcf84d6..a296db0c32 100644
> --- a/include/vlc_fixups.h
> +++ b/include/vlc_fixups.h
> @@ -660,6 +660,9 @@ FILE *vlc_win32_tmpfile(void);
> # define fdatasync fsync
> 
> # include <time.h>
> +# ifndef TIMER_ABSTIME
> +#  define TIMER_ABSTIME 0x01
> +# endif
> # ifndef CLOCK_REALTIME
> #  define CLOCK_REALTIME 0
> # endif
> @@ -669,6 +672,10 @@ FILE *vlc_win32_tmpfile(void);
> # ifndef HAVE_CLOCK_GETTIME
> int clock_gettime(clockid_t clock_id, struct timespec *tp);
> # endif
> +# ifndef HAVE_CLOCK_NANOSLEEP
> +int clock_nanosleep(clockid_t clock_id, int flags,
> +        const struct timespec *rqtp, struct timespec *rmtp);
> +# endif
> #endif
> 
> #ifdef __cplusplus
> -- 
> 2.21.1 (Apple Git-122.3)
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel