Re: [Qemu-devel] [PATCH 2/3] Don't exit() in config_error()

[Mark McLoughlin]

On Mon, 2009-09-28 at 21:11 +0200, Markus Armbruster wrote:
> Propagating errors up the call chain is tedious.  In startup code, we
> can take a shortcut: terminate the program.  This is wrong elsewhere,
> the monitor in particular.
> 
> config_error() tries to cater for both customers: it terminates the
> program unless its mon parameter tells it it's working for the
> monitor.
> 
> Its users need to return status anyway (unless passing a null mon
> argument, which none do), which their users need to check.  So this
> automatic exit buys us exactly nothing useful.  Only the dangerous
> delusion that we can get away without returning status.  Some of its
> users fell for that.  Their callers continue executing after failure
> when working for the monitor.
> 
> This bites monitor command host_net_add in two places:
> 
> * net_slirp_init() continues after slirp_hostfwd(), slirp_guestfwd(),
>   or slirp_smb() failed, and may end up reporting success.  This
>   happens for "host_net_add user guestfwd=foo": it complains about the
>   invalid guest forwarding rule, then happily creates the user network
>   without guest forwarding.
> 
> * net_client_init() can't detect slirp_guestfwd() failure, and gets
>   fooled by net_slirp_init() lying about success.  Suppresses its
>   "Could not initialize device" message.
> 
> Add the missing error reporting, make sure errors are checked, and
> drop the exit() from config_error().
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  net.c |   83 ++++++++++++++++++++++++++++++++++++----------------------------
>  net.h |    4 +-
>  vl.c  |    9 ++++--
>  3 files changed, 55 insertions(+), 41 deletions(-)
> 
...
> diff --git a/vl.c b/vl.c
> index 7bfd415..96e4312 100644
> --- a/vl.c
> +++ b/vl.c
...
> @@ -5766,7 +5768,8 @@ int main(int argc, char **argv, char **envp)
>  
>      /* init USB devices */
>      if (usb_enabled) {
> -        foreach_device_config(DEV_USB, usb_parse);
> +        if (foreach_device_config(DEV_USB, usb_parse) < 0)
> +            exit(1);
>      }
>  
>      /* init generic devices */

This hunk appears to be unrelated

Cheers,
Mark.