[pulseaudio-discuss] [PATCH] pipe-sink: new option "use_existing_fifo"

[Georg Chini]

On 09.01.2018 22:24, Samo Pogačnik wrote:
> Allow usage of an already existing fifo (named pipe) upon module
> load. Also, the fifo is not going to be removed upon module unload,
> if "use_existing_fifo" option has been enabled.

I wonder if we really need a new option for this. Can't you just check
if the file exists and use it if it is present? You could set a 
do_not_unlink
flag if it exists to avoid removing an already existing file on exit.

> ---
>   src/modules/module-pipe-sink.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/src/modules/module-pipe-sink.c b/src/modules/module-pipe-sink.c
> index fe2af70..eafc3f7 100644
> --- a/src/modules/module-pipe-sink.c
> +++ b/src/modules/module-pipe-sink.c
> @@ -62,6 +62,7 @@ PA_MODULE_USAGE(
>           "channels=<number of channels> "
>           "channel_map=<channel map> "
>           "use_system_clock_for_timing=<yes or no> "
> +        "use_existing_fifo=<yes or no> "
>   );
>   
>   #define DEFAULT_FILE_NAME "fifo_output"
> @@ -91,6 +92,7 @@ struct userdata {
>       pa_usec_t timestamp;
>   
>       bool use_system_clock_for_timing;
> +    bool use_existing_fifo;
>   };
>   
>   static const char* const valid_modargs[] = {
> @@ -102,6 +104,7 @@ static const char* const valid_modargs[] = {
>       "channels",
>       "channel_map",
>       "use_system_clock_for_timing",
> +    "use_existing_fifo",
>       NULL
>   };
>   
> @@ -442,6 +445,11 @@ int pa__init(pa_module *m) {
>           goto fail;
>       }
>   
> +    if (pa_modargs_get_value_boolean(ma, "use_existing_fifo", &u->use_existing_fifo) < 0) {
> +        pa_log("Failed to parse use_existing_fifo argument.");
> +        goto fail;
> +    }
> +
>       if (pa_thread_mq_init(&u->thread_mq, m->core->mainloop, u->rtpoll) < 0) {
>           pa_log("pa_thread_mq_init() failed.");
>           goto fail;
> @@ -452,8 +460,10 @@ int pa__init(pa_module *m) {
>       u->filename = pa_runtime_path(pa_modargs_get_value(ma, "file", DEFAULT_FILE_NAME));
>   
>       if (mkfifo(u->filename, 0666) < 0) {
> -        pa_log("mkfifo('%s'): %s", u->filename, pa_cstrerror(errno));
> -        goto fail;
> +        int errno_save = errno;
> +        pa_log("mkfifo('%s'): %s", u->filename, pa_cstrerror(errno_save));
> +        if (!u->use_existing_fifo || errno_save != EEXIST)
> +            goto fail;
>       }

The log line should be within the if condition. Also, should we not make 
sure that
the file is a pipe if it exists?

>       if ((u->fd = pa_open_cloexec(u->filename, O_RDWR, 0)) < 0) {
>           pa_log("open('%s'): %s", u->filename, pa_cstrerror(errno));
> @@ -584,7 +594,8 @@ void pa__done(pa_module *m) {
>           pa_rtpoll_free(u->rtpoll);
>   
>       if (u->filename) {
> -        unlink(u->filename);
> +        if (!u->use_existing_fifo)
> +            unlink(u->filename);
>           pa_xfree(u->filename);
>       }
>