diff mbox series

[v2] audio/pw: Report more accurate error when connecting to PipeWire fails

Message ID 3a78811ad5b0e87816b7616ab21d2eeef00b9c52.1726647033.git.mprivozn@redhat.com (mailing list archive)
State New, archived
Headers show
Series [v2] audio/pw: Report more accurate error when connecting to PipeWire fails | expand

Commit Message

Michal Privoznik Sept. 18, 2024, 8:17 a.m. UTC 
According to its man page [1], pw_context_connect() sets errno on
failure:

  Returns a Core on success or NULL with errno set on error.

It may be handy to see errno when figuring out why PipeWire
failed to connect. That leaves us with just one possible path to
reach 'fail_error' label which is then moved to that path and
also its error message is adjusted slightly.

1: https://docs.pipewire.org/group__pw__core.html#ga5994e3a54e4ec718094ca02a1234815b

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---

v2 of:

https://mail.gnu.org/archive/html/qemu-devel/2024-09/msg03485.html

Comments

Marc-André Lureau Sept. 18, 2024, 9:29 a.m. UTC | #1 
On Wed, Sep 18, 2024 at 12:17 PM Michal Privoznik <mprivozn@redhat.com> wrote:
>
> According to its man page [1], pw_context_connect() sets errno on
> failure:
>
>   Returns a Core on success or NULL with errno set on error.
>
> It may be handy to see errno when figuring out why PipeWire
> failed to connect. That leaves us with just one possible path to
> reach 'fail_error' label which is then moved to that path and
> also its error message is adjusted slightly.
>
> 1: https://docs.pipewire.org/group__pw__core.html#ga5994e3a54e4ec718094ca02a1234815b
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>
> v2 of:
>
> https://mail.gnu.org/archive/html/qemu-devel/2024-09/msg03485.html
>
> diff to v1:
> - Dropped duplicated S-o-b line
> - Fixed typo on error message (s/PipeWite/PipeWire/)
>
>  audio/pwaudio.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/audio/pwaudio.c b/audio/pwaudio.c
> index 3b14e04fbb..8e13b58286 100644
> --- a/audio/pwaudio.c
> +++ b/audio/pwaudio.c
> @@ -769,13 +769,15 @@ qpw_audio_init(Audiodev *dev, Error **errp)
>      pw->core = pw_context_connect(pw->context, NULL, 0);
>      if (pw->core == NULL) {
>          pw_thread_loop_unlock(pw->thread_loop);
> -        goto fail_error;
> +        error_setg_errno(errp, errno, "Failed to connect to PipeWire instance");
> +        goto fail;
>      }
>
>      if (pw_core_add_listener(pw->core, &pw->core_listener,
>                               &core_events, pw) < 0) {
>          pw_thread_loop_unlock(pw->thread_loop);
> -        goto fail_error;
> +        error_setg(errp, "Failed to add PipeWire listener");
> +        goto fail;
>      }
>      if (wait_resync(pw) < 0) {
>          pw_thread_loop_unlock(pw->thread_loop);
> @@ -785,8 +787,6 @@ qpw_audio_init(Audiodev *dev, Error **errp)
>
>      return g_steal_pointer(&pw);
>
> -fail_error:
> -    error_setg(errp, "Failed to initialize PW context");
>  fail:
>      if (pw->thread_loop) {
>          pw_thread_loop_stop(pw->thread_loop);
> --
> 2.44.2
>
Manos Pitsidianakis Sept. 18, 2024, 11:23 a.m. UTC | #2 
On Wed, 18 Sep 2024 11:17, Michal Privoznik <mprivozn@redhat.com> wrote:
>According to its man page [1], pw_context_connect() sets errno on
>failure:
>
>  Returns a Core on success or NULL with errno set on error.
>
>It may be handy to see errno when figuring out why PipeWire
>failed to connect. That leaves us with just one possible path to
>reach 'fail_error' label which is then moved to that path and
>also its error message is adjusted slightly.
>
>1: https://docs.pipewire.org/group__pw__core.html#ga5994e3a54e4ec718094ca02a1234815b
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
>
>v2 of:
>
>https://mail.gnu.org/archive/html/qemu-devel/2024-09/msg03485.html
>
>diff to v1:
>- Dropped duplicated S-o-b line
>- Fixed typo on error message (s/PipeWite/PipeWire/)
>
> audio/pwaudio.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/audio/pwaudio.c b/audio/pwaudio.c
>index 3b14e04fbb..8e13b58286 100644
>--- a/audio/pwaudio.c
>+++ b/audio/pwaudio.c
>@@ -769,13 +769,15 @@ qpw_audio_init(Audiodev *dev, Error **errp)
>     pw->core = pw_context_connect(pw->context, NULL, 0);
>     if (pw->core == NULL) {
>         pw_thread_loop_unlock(pw->thread_loop);
>-        goto fail_error;
>+        error_setg_errno(errp, errno, "Failed to connect to PipeWire instance");
>+        goto fail;
>     }
> 
>     if (pw_core_add_listener(pw->core, &pw->core_listener,
>                              &core_events, pw) < 0) {
>         pw_thread_loop_unlock(pw->thread_loop);
>-        goto fail_error;
>+        error_setg(errp, "Failed to add PipeWire listener");
>+        goto fail;
>     }
>     if (wait_resync(pw) < 0) {
>         pw_thread_loop_unlock(pw->thread_loop);
>@@ -785,8 +787,6 @@ qpw_audio_init(Audiodev *dev, Error **errp)
> 
>     return g_steal_pointer(&pw);
> 
>-fail_error:
>-    error_setg(errp, "Failed to initialize PW context");
> fail:
>     if (pw->thread_loop) {
>         pw_thread_loop_stop(pw->thread_loop);
>-- 
>2.44.2
>
>


Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Manos Pitsidianakis Sept. 18, 2024, 11:23 a.m. UTC | #3 
On Wed, 18 Sep 2024 11:17, Michal Privoznik <mprivozn@redhat.com> wrote:
>According to its man page [1], pw_context_connect() sets errno on
>failure:
>
>  Returns a Core on success or NULL with errno set on error.
>
>It may be handy to see errno when figuring out why PipeWire
>failed to connect. That leaves us with just one possible path to
>reach 'fail_error' label which is then moved to that path and
>also its error message is adjusted slightly.
>
>1: https://docs.pipewire.org/group__pw__core.html#ga5994e3a54e4ec718094ca02a1234815b
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
>
>v2 of:
>
>https://mail.gnu.org/archive/html/qemu-devel/2024-09/msg03485.html
>
>diff to v1:
>- Dropped duplicated S-o-b line
>- Fixed typo on error message (s/PipeWite/PipeWire/)
>
> audio/pwaudio.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/audio/pwaudio.c b/audio/pwaudio.c
>index 3b14e04fbb..8e13b58286 100644
>--- a/audio/pwaudio.c
>+++ b/audio/pwaudio.c
>@@ -769,13 +769,15 @@ qpw_audio_init(Audiodev *dev, Error **errp)
>     pw->core = pw_context_connect(pw->context, NULL, 0);
>     if (pw->core == NULL) {
>         pw_thread_loop_unlock(pw->thread_loop);
>-        goto fail_error;
>+        error_setg_errno(errp, errno, "Failed to connect to PipeWire instance");
>+        goto fail;
>     }
> 
>     if (pw_core_add_listener(pw->core, &pw->core_listener,
>                              &core_events, pw) < 0) {
>         pw_thread_loop_unlock(pw->thread_loop);
>-        goto fail_error;
>+        error_setg(errp, "Failed to add PipeWire listener");
>+        goto fail;
>     }
>     if (wait_resync(pw) < 0) {
>         pw_thread_loop_unlock(pw->thread_loop);
>@@ -785,8 +787,6 @@ qpw_audio_init(Audiodev *dev, Error **errp)
> 
>     return g_steal_pointer(&pw);
> 
>-fail_error:
>-    error_setg(errp, "Failed to initialize PW context");
> fail:
>     if (pw->thread_loop) {
>         pw_thread_loop_stop(pw->thread_loop);
>-- 
>2.44.2
>
>


Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Michal Privoznik Oct. 10, 2024, 7:09 a.m. UTC | #4 
On 9/18/24 11:29, Marc-André Lureau wrote:
> On Wed, Sep 18, 2024 at 12:17 PM Michal Privoznik <mprivozn@redhat.com> wrote:
>>
>> According to its man page [1], pw_context_connect() sets errno on
>> failure:
>>
>>   Returns a Core on success or NULL with errno set on error.
>>
>> It may be handy to see errno when figuring out why PipeWire
>> failed to connect. That leaves us with just one possible path to
>> reach 'fail_error' label which is then moved to that path and
>> also its error message is adjusted slightly.
>>
>> 1: https://docs.pipewire.org/group__pw__core.html#ga5994e3a54e4ec718094ca02a1234815b
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks, can you merge it too please? I don't have commit access.

Michal
diff mbox series

Patch

diff to v1:
- Dropped duplicated S-o-b line
- Fixed typo on error message (s/PipeWite/PipeWire/)

 audio/pwaudio.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/audio/pwaudio.c b/audio/pwaudio.c
index 3b14e04fbb..8e13b58286 100644
--- a/audio/pwaudio.c
+++ b/audio/pwaudio.c
@@ -769,13 +769,15 @@  qpw_audio_init(Audiodev *dev, Error **errp)
     pw->core = pw_context_connect(pw->context, NULL, 0);
     if (pw->core == NULL) {
         pw_thread_loop_unlock(pw->thread_loop);
-        goto fail_error;
+        error_setg_errno(errp, errno, "Failed to connect to PipeWire instance");
+        goto fail;
     }
 
     if (pw_core_add_listener(pw->core, &pw->core_listener,
                              &core_events, pw) < 0) {
         pw_thread_loop_unlock(pw->thread_loop);
-        goto fail_error;
+        error_setg(errp, "Failed to add PipeWire listener");
+        goto fail;
     }
     if (wait_resync(pw) < 0) {
         pw_thread_loop_unlock(pw->thread_loop);
@@ -785,8 +787,6 @@  qpw_audio_init(Audiodev *dev, Error **errp)
 
     return g_steal_pointer(&pw);
 
-fail_error:
-    error_setg(errp, "Failed to initialize PW context");
 fail:
     if (pw->thread_loop) {
         pw_thread_loop_stop(pw->thread_loop);