diff mbox series

[18/18] audio/spiceaudio: Fail initialisation when not using spice

Message ID 5db1fdef0330f20ed6ae306b5a71dad1b5e9b44c.1650874791.git.mkletzan@redhat.com
State New
Headers show
Series RFC: Remove deprecated audio features | expand

Commit Message

Martin Kletzander April 25, 2022, 8:22 a.m. UTC 
The caller would already fail, but this way the message can better
express the reason for the failure.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2043498

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 audio/spiceaudio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Daniel P. Berrangé April 25, 2022, 1:08 p.m. UTC | #1 
On Mon, Apr 25, 2022 at 10:22:01AM +0200, Martin Kletzander wrote:
> The caller would already fail, but this way the message can better
> express the reason for the failure.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2043498
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  audio/spiceaudio.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
> index a8d370fe6f31..fdbd7dc285ad 100644
> --- a/audio/spiceaudio.c
> +++ b/audio/spiceaudio.c
> @@ -74,8 +74,9 @@ static const SpiceRecordInterface record_sif = {
>  static void *spice_audio_init(Audiodev *dev)
>  {
>      if (!using_spice) {
> -        return NULL;
> +        error_setg(&error_fatal, "Cannot use spice audio without -spice");

Typically one would not use error_fatal directly with a call
to error_setg(). The usual pattern would be for the method
calling error_setg() to have an 'Error **errp' parameter.
The caller would then pass in &error_fatal when calling the
method, or pass in a real error object if wishing to receive
the error.

If you don't want to plumb in an 'Error **errp' to the
spice_audio_init() method, then it would be sufficient to
instead just do

   error_report("Cannot use spice....")

Using 'Error **errp' is best practice in new code, but no one
will blame you for not refactoring existing code to support
this if looks like too much work.

With regards,
Daniel
diff mbox series

Patch

diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index a8d370fe6f31..fdbd7dc285ad 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -74,8 +74,9 @@  static const SpiceRecordInterface record_sif = {
 static void *spice_audio_init(Audiodev *dev)
 {
     if (!using_spice) {
-        return NULL;
+        error_setg(&error_fatal, "Cannot use spice audio without -spice");
     }
+
     return &spice_audio_init;
 }