Re: [PATCH v2 18/37] bsd-user: Add utimes, futimes, and ktimer functions to os-time.h

On Fri, May 22, 2026 at 5:38 PM Pierrick Bouvier <
[email protected]> wrote:

> On 5/18/2026 2:27 PM, Warner Losh wrote:
> > Add time-related system call shims: utimes, lutimes, futimes,
> > futimesat, ktimer_create, ktimer_delete, ktimer_settime, and
> > ktimer_gettime.
> >
> > Signed-off-by: Stacey Son <[email protected]>
> > Signed-off-by: Mikaël Urankar <[email protected]>
> > Signed-off-by: Kyle Evans <[email protected]>
> > Signed-off-by: Warner Losh <[email protected]>
> > Assisted-by: Claude Opus 4.6 (1M context)
> > ---
> >  bsd-user/freebsd/os-time.h | 210
> +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 210 insertions(+)
> >
> > diff --git a/bsd-user/freebsd/os-time.h b/bsd-user/freebsd/os-time.h
> > index 16b71e6234..05fa043442 100644
> > --- a/bsd-user/freebsd/os-time.h
> > +++ b/bsd-user/freebsd/os-time.h
> > @@ -217,5 +217,215 @@ static inline abi_long
> do_freebsd_ntp_gettime(abi_ulong target_ntv_addr)
> >  }
> >
> >
> > +/* utimes(2) */
> > +static inline abi_long do_freebsd_utimes(abi_long arg1, abi_long arg2)
> > +{
> > +    abi_long ret;
> > +    void *p;
> > +    struct timeval *tvp, tv[2];
> > +
> > +    if (arg2 != 0) {
> > +        if (t2h_freebsd_timeval(&tv[0], arg2) ||
> > +                t2h_freebsd_timeval(&tv[1], arg2 +
> > +                        sizeof(struct target_freebsd_timeval))) {
> > +            return -TARGET_EFAULT;
> > +        }
> > +        tvp = tv;
> > +    } else {
> > +        tvp = NULL;
> > +    }
> > +    p = lock_user_string(arg1);
> > +    if (p == NULL) {
> > +        return -TARGET_EFAULT;
> > +    }
> > +    ret = get_errno(utimes(p, tvp));
> > +    unlock_user(p, arg1, 0);
> > +
> > +    return ret;
> > +}
> > +
> > +/* lutimes(2) */
> > +static inline abi_long do_freebsd_lutimes(abi_long arg1, abi_long arg2)
> > +{
> > +    abi_long ret;
> > +    void *p;
> > +    struct timeval *tvp, tv[2];
> > +
> > +    if (arg2 != 0) {
> > +        if (t2h_freebsd_timeval(&tv[0], arg2) ||
> > +                t2h_freebsd_timeval(&tv[1], arg2 +
> > +                        sizeof(struct target_freebsd_timeval))) {
> > +            return -TARGET_EFAULT;
> > +        }
> > +        tvp = tv;
> > +    } else {
> > +        tvp = NULL;
> > +    }
> > +    p = lock_user_string(arg1);
> > +    if (p == NULL) {
> > +        return -TARGET_EFAULT;
> > +    }
> > +    ret = get_errno(lutimes(p, tvp));
> > +    unlock_user(p, arg1, 0);
> > +
> > +    return ret;
> > +}
> > +
> > +/* futimes(2) */
> > +static inline abi_long do_freebsd_futimes(abi_long arg1, abi_long arg2)
> > +{
> > +    struct timeval *tvp, tv[2];
> > +
> > +    if (arg2 != 0) {
> > +        if (t2h_freebsd_timeval(&tv[0], arg2) ||
> > +                t2h_freebsd_timeval(&tv[1], arg2 +
> > +                        sizeof(struct target_freebsd_timeval))) {
> > +            return -TARGET_EFAULT;
> > +        }
> > +        tvp = tv;
> > +    } else {
> > +        tvp = NULL;
> > +    }
> > +
> > +    return get_errno(futimes(arg1, tvp));
> > +}
> > +
> > +/* futimesat(2) */
> > +static inline abi_long do_freebsd_futimesat(abi_long arg1, abi_long
> arg2,
> > +        abi_long arg3)
> > +{
> > +    abi_long ret;
> > +    void *p;
> > +    struct timeval *tvp, tv[2];
> > +
> > +    if (arg3 != 0) {
> > +        if (t2h_freebsd_timeval(&tv[0], arg3) ||
> > +                t2h_freebsd_timeval(&tv[1], arg3 +
> > +                        sizeof(struct target_freebsd_timeval))) {
> > +            return -TARGET_EFAULT;
> > +        }
> > +        tvp = tv;
> > +    } else {
> > +        tvp = NULL;
> > +    }
> > +
> > +    p = lock_user_string(arg2);
> > +    if (p == NULL) {
> > +        return -TARGET_EFAULT;
> > +    }
> > +    ret = get_errno(futimesat(arg1, p, tvp));
> > +    unlock_user(p, arg2, 0);
> > +
> > +    return ret;
> > +}
> > +
>
> The functions above are quite duplicated.
> We could factor the first part at least, setting tvp.
> The lock_user_string part can be replaced with LOCK_PATH.
>

This is a good suggestion. I hesitated to implement it when I last had time
to work
on bsd-user upstreaming. I've created
https://github.com/qemu-bsd-user/qemu-bsd-user/issues/65 since I'd like to
defer
changes here. There's no good regression tests for these functions and I'd
like
to do the refactoring when I have the time to focus on ensuring they
continue to
be correct. In all the rebasing, I've somehow created a hang that I can't
recreate
in a small test case sometime during the last several years. I'd like to
get what
we've been using upstreamed and then focus on improving the code.

Warner


> > +/* timer_create(2) */
> > +static inline abi_long do_freebsd_ktimer_create(abi_long arg1, abi_long
> arg2,
> > +        abi_long arg3)
> > +{
> > +    /* args: clockid_t clockid, struct sigevent *sevp, int *timerid */
> > +    abi_long ret;
> > +
> > +    struct sigevent host_sevp = { 0 }, *phost_sevp = NULL;
> > +
> > +    int clkid = arg1;
> > +    int timer_index = next_free_host_timer();
> > +
> > +    if (timer_index < 0) {
> > +        ret = -TARGET_EAGAIN;
> > +    } else {
> > +        int *phtimer = g_posix_timers  + timer_index;
> > +
> > +        if (arg2) {
> > +            phost_sevp = &host_sevp;
> > +            ret = target_to_host_sigevent(phost_sevp, arg2);
> > +            if (ret != 0) {
> > +                return -TARGET_EFAULT;
> > +            }
> > +        }
> > +
> > +        ret = get_errno(__sys_ktimer_create(clkid, phost_sevp,
> phtimer));
> > +        if (ret) {
> > +            phtimer = NULL;
> > +        } else {
> > +            if (put_user(TIMER_MAGIC | timer_index, arg3, int)) {
> > +                ret = -TARGET_EFAULT;
> > +            }
> > +        }
> > +    }
> > +    return ret;
> > +}
> > +
> > +/* timer_delete(2) */
> > +static inline abi_long do_freebsd_ktimer_delete(abi_long arg1)
> > +{
> > +    /* args: int timerid */
> > +    abi_long ret;
> > +    int timerid = get_timer_id(arg1);
> > +
> > +    if (timerid < 0) {
> > +        ret = timerid;
> > +    } else {
> > +        int htimer = g_posix_timers[timerid];
> > +        ret = get_errno(__sys_ktimer_delete(htimer));
> > +        g_posix_timers[timerid] = 0;
> > +    }
> > +    return ret;
> > +}
> > +
> > +/* timer_settime(2) */
> > +static inline abi_long do_freebsd_ktimer_settime(abi_long arg1,
> abi_long arg2,
> > +        abi_long arg3, abi_long arg4)
> > +{
> > +    /*
> > +     * args: int timerid, int flags, const struct itimerspec *new_value,
> > +     * struct itimerspec * old_value
> > +     */
> > +    abi_long ret;
> > +    int timerid = get_timer_id(arg1);
> > +
> > +    if (timerid < 0) {
> > +        ret = timerid;
> > +    } else if (arg3 == 0) {
> > +        ret = -TARGET_EINVAL;
> > +    } else {
> > +        int htimer = g_posix_timers[timerid];
> > +        struct itimerspec hspec_new = {{0},}, hspec_old = {{0},};
> > +
> > +        if (target_to_host_itimerspec(&hspec_new, arg3)) {
> > +            return -TARGET_EFAULT;
> > +        }
> > +        ret = get_errno(
> > +            __sys_ktimer_settime(htimer, arg2, &hspec_new, &hspec_old));
> > +        if (arg4 && host_to_target_itimerspec(arg4, &hspec_old)) {
> > +            return -TARGET_EFAULT;
> > +        }
> > +    }
> > +    return ret;
> > +}
> > +
> > +/* timer_gettime(2) */
> > +static inline abi_long do_freebsd_ktimer_gettime(abi_long arg1,
> abi_long arg2)
> > +{
> > +    /* args: int timerid, struct itimerspec *curr_value */
> > +    abi_long ret;
> > +    int timerid = get_timer_id(arg1);
> > +
> > +    if (timerid < 0) {
> > +        ret = timerid;
> > +    } else if (!arg2) {
> > +        ret = -TARGET_EFAULT;
> > +    } else {
> > +        int htimer = g_posix_timers[timerid];
> > +        struct itimerspec hspec;
> > +        ret = get_errno(__sys_ktimer_gettime(htimer, &hspec));
> > +
> > +        if (host_to_target_itimerspec(arg2, &hspec)) {
> > +            ret = -TARGET_EFAULT;
> > +        }
> > +    }
> > +    return ret;
> > +}
> > +
> >
> >  #endif /* FREEBSD_OS_TIME_H */
> >
>
>