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

[Marvin Scholz]

On 20 Mar 2020, at 19:05, David Fuhrmann wrote:

>> 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
>

Hi, you are right, removed the macros.

>> +#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
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel