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

[Daniel P . Berrangé]

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|