[vlc-devel] [PATCH 1/6] vlc_spawn: vector posix_spawn/fork code

[Marvin Scholz]

Hi!
Did not do a full review as I am not very familiar
with the spawn APIs usage in VLC. Overall seem like a good
change to do though.

Just a quick remark about a documentation block below:

On 19 Sep 2020, at 12:29, Rémi Denis-Courmont wrote:

> This adds vlc_spawn(), vlc_spawnp() and vlc_waitpid() to factor the
> common code around the posix_spawn() family. It is added as part of
> the plugin API so that core code can use if (namely 
> vlc_getProxyUrl()),
> which would not be possible with a shared C source in modules/.
>
> Win32 back-end may be provided later, e.g. based on old vlc_execve()
> implementation, if/when this actually gets used on Windows.
> ---
>  include/vlc_spawn.h |  87 ++++++++++++++++++++++++++
>  src/Makefile.am     |   2 +
>  src/libvlccore.sym  |   3 +
>  src/missing.c       |  26 ++++++++
>  src/posix/spawn.c   | 147 
> ++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 265 insertions(+)
>  create mode 100644 include/vlc_spawn.h
>  create mode 100644 src/posix/spawn.c
>
> diff --git a/include/vlc_spawn.h b/include/vlc_spawn.h
> new file mode 100644
> index 0000000000..098561e9b1
> --- /dev/null
> +++ b/include/vlc_spawn.h
> @@ -0,0 +1,87 @@
> +/*****************************************************************************
> + * vlc_spawn.h: Child process helpers
> + 
> *****************************************************************************
> + * Copyright © 2020 Rémi Denis-Courmont
> + *
> + * 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.
> + 
> *****************************************************************************/
> +
> +#ifndef VLC_SPAWN_H
> +#define VLC_SPAWN_H 1
> +
> +#include <sys/types.h>
> +
> +/**
> + * \defgroup spawn Process management
> + * \ingroup os
> + * @{
> + */
> +
> +/**
> + * Spawn a child process (by file name).
> + *
> + * This function creates a child process. For all practical purposes, 
> it is
> + * identical to vlc_spawnp(), with one exception: vlc_spawn() does 
> <b>not</b>

I think this lacks a word? Maybe it's meant to say "does not _search_ 
for…"?

> + * for the executable file name in the OS-defined search path. 
> Instead the
> + * file name must be absolute.
> + */
> +VLC_API VLC_USED
> +int vlc_spawn(pid_t *pid, const char *file, const int *fdv,
> +              const char *const *argv);
> +
> +/**
> + * Spawn a child process.
> + *
> + * This function creates a child process. The created process will 
> run with the
> + * same environment and privileges as the calling process.
> + *
> + * An array of file descriptors must be provided for the child 
> process:
> + * - fdv[0] is the standard input (or -1 for nul input),
> + * - fdv[1] is the standard output (or -1 for nul output),
> + * - fdv[2] is the standard error (or -1 to ignore).
> + * The array must have at least 4 elements and be terminated by -1.
> + * Some platforms will ignore file descriptors other than the three 
> standard
> + * ones, so those should not be used in code intended to be portable.
> + *
> + * vlc_waitpid() must be called to collect outstanding system 
> resources
> + * after the child process terminates.
> + *
> + * \param pid storage space for the child process identifier [OUT]
> + * \param path executable path [IN]
> + * \param fdv file descriptor array [IN]
> + * \param argv NULL-terminated array of command line arguments [IN]
> + *
> + * \return 0 on success or a standard error code on failure
> + */
> +VLC_API VLC_USED
> +int vlc_spawnp(pid_t *pid, const char *path, const int *fdv,
> +              const char *const *argv);
> +
> +/**
> + * Waits for a child process.
> + *
> + * This function waits until a child process created by vlc_spawn() 
> or
> + * vlc_spawnp() has completed and releases any associated system 
> resource.
> + *
> + * \param pid process identifier as returned by vlc_spawn() or 
> vlc_spawnp()
> + *
> + * \return This function returns the process exit code.
> + */
> +VLC_API
> +int vlc_waitpid(pid_t pid);
> +
> +/** @} */
> +
> +#endif
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 51d8600e28..d8196fca61 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -91,6 +91,7 @@ pluginsinclude_HEADERS = \
>  	../include/vlc_services_discovery.h \
>  	../include/vlc_sort.h \
>  	../include/vlc_sout.h \
> +	../include/vlc_spawn.h \
>  	../include/vlc_spu.h \
>  	../include/vlc_stream.h \
>  	../include/vlc_stream_extractor.h \
> @@ -478,6 +479,7 @@ libvlccore_la_SOURCES += \
>  	posix/filesystem.c \
>  	posix/plugin.c \
>  	posix/rand.c \
> +	posix/spawn.c \
>  	posix/timer.c
>  if !HAVE_LINUX
>  libvlccore_la_SOURCES += posix/wait.c
> diff --git a/src/libvlccore.sym b/src/libvlccore.sym
> index 31fcd37cba..023203ea4f 100644
> --- a/src/libvlccore.sym
> +++ b/src/libvlccore.sym
> @@ -450,6 +450,9 @@ vlc_accept
>  vlc_send
>  vlc_sendto
>  vlc_sendmsg
> +vlc_spawn
> +vlc_spawnp
> +vlc_waitpid
>  utf8_vfprintf
>  var_AddCallback
>  var_AddListCallback
> diff --git a/src/missing.c b/src/missing.c
> index 34283b63eb..45fddf177f 100644
> --- a/src/missing.c
> +++ b/src/missing.c
> @@ -284,3 +284,29 @@ noreturn void vlc_control_cancel (vlc_cleanup_t 
> *cleaner)
>      vlc_assert_unreachable ();
>  }
>  #endif
> +
> +#include <errno.h>
> +#include <vlc_spawn.h>
> +
> +VLC_WEAK
> +int vlc_spawn(pid_t *pid, const char *file, const int *fds,
> +              const char *const *args)
> +{
> +    (void) pid; (void) file; (void) fds; (void) args;
> +    return ENOSYS;
> +}
> +
> +VLC_WEAK
> +int vlc_spawnp(pid_t *pid, const char *path, const int *fds,
> +               const char *const *args)
> +{
> +    (void) pid; (void) path; (void) fds; (void) args;
> +    return ENOSYS;
> +}
> +
> +VLC_WEAK
> +int vlc_waitpid(pid_t pid)
> +{
> +    (void) pid;
> +    vlc_assert_unreachable();
> +}
> diff --git a/src/posix/spawn.c b/src/posix/spawn.c
> new file mode 100644
> index 0000000000..774c6cb6cb
> --- /dev/null
> +++ b/src/posix/spawn.c
> @@ -0,0 +1,147 @@
> +/*****************************************************************************
> + * spawn.c
> + 
> *****************************************************************************
> + * Copyright (C) 2008, 2020 Rémi Denis-Courmont
> + *
> + * 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 HAVE_CONFIG_H
> +# include "config.h"
> +#endif
> +
> +#include <assert.h>
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <signal.h>
> +#include <sys/wait.h>
> +
> +#include <vlc_common.h>
> +#include <vlc_spawn.h>
> +
> +#if (_POSIX_SPAWN >= 0)
> +#include <spawn.h>
> +
> +extern char **environ;
> +
> +static int vlc_spawn_inner(pid_t *restrict pid, const char *path,
> +                           const int *fdv, const char *const *argv,
> +                           bool search)
> +{
> +    posix_spawnattr_t attr;
> +    posix_spawn_file_actions_t fas;
> +    char **vargv;
> +    size_t argc = 0;
> +    int err;
> +
> +    assert(pid != NULL);
> +    assert(path != NULL);
> +    assert(fdv != NULL);
> +    assert(argv != NULL);
> +
> +    err = posix_spawn_file_actions_init(&fas);
> +    if (unlikely(err != 0))
> +        return err;
> +
> +    for (int newfd = 0; newfd < 3 || fdv[newfd] != -1; newfd++) {
> +        int oldfd = fdv[newfd];
> +
> +        if (oldfd == -1)
> +            err = posix_spawn_file_actions_addopen(&fas, newfd, 
> "/dev/null",
> +                                                   O_RDWR, 0644);
> +        else
> +            err = posix_spawn_file_actions_adddup2(&fas, oldfd, 
> newfd);
> +
> +        if (unlikely(err != 0)) {
> +            posix_spawn_file_actions_destroy(&fas);
> +            return err;
> +        }
> +    }
> +
> +    posix_spawnattr_init(&attr);
> +    {
> +        sigset_t set;
> +
> +        /* Unmask signals. */
> +        sigemptyset(&set);
> +        posix_spawnattr_setsigmask(&attr, &set);
> +
> +        /* Reset SIGPIPE handler (which VLC overrode). */
> +        sigaddset(&set, SIGPIPE);
> +        posix_spawnattr_setsigdefault(&attr, &set);
> +    }
> +    posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSIGDEF
> +                                  | POSIX_SPAWN_SETSIGMASK);
> +
> +    /* For hysterical raisins, POSIX uses non-const character 
> pointers.
> +     * We need to copy manually due to aliasing rules.
> +     */
> +    while (argv[argc++] != NULL);
> +
> +    vargv = malloc(sizeof (*argv) * argc);
> +    if (unlikely(vargv == NULL)) {
> +        err = errno;
> +        goto out;
> +    }
> +
> +    for (size_t i = 0; i < argc; i++)
> +        vargv[i] = (char *)argv[i];
> +
> +    if (search)
> +        err = posix_spawnp(pid, path, &fas, &attr, vargv, environ);
> +    else
> +        err = posix_spawn(pid, path, &fas, &attr, vargv, environ);
> +
> +out:
> +    free(vargv);
> +    posix_spawnattr_destroy(&attr);
> +    posix_spawn_file_actions_destroy(&fas);
> +    return err;
> +}
> +
> +#else /* _POSIX_SPAWN */
> +
> +static int vlc_spawn_inner(pid_t *restrict pid, const char *path,
> +                           const int *fdv, const char *const *argv,
> +                           bool search)
> +{
> +    (void) pid; (void) path; (void) fdv; (void) argv; (void) search;
> +}
> +
> +#endif /* _POSIX_SPAWN */
> +
> +int vlc_spawnp(pid_t *restrict pid, const char *path,
> +               const int *fdv, const char *const *argv)
> +{
> +    return vlc_spawn_inner(pid, path, fdv, argv, true);
> +}
> +
> +int vlc_spawn(pid_t *restrict pid, const char *file,
> +              const int *fdv, const char *const *argv)
> +{
> +    return vlc_spawn_inner(pid, file, fdv, argv, false);
> +}
> +
> +int vlc_waitpid(pid_t pid)
> +{
> +    int status;
> +
> +    while (waitpid(pid, &status, 0) == -1)
> +        assert(errno != ECHILD && errno != EINVAL);
> +
> +    return status;
> +}
> -- 
> 2.28.0
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel