Re: [PATCH v19 01/15] net/tap: rework tap_parse_script

[Markus Armbruster <armbru@redhat.com>]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> Factor out tap_is_explicit_no_script() helper, to simplify
> further changes.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  net/tap.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/net/tap.c b/net/tap.c
> index 57ffb09885c..fedd48c48d2 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -92,19 +92,34 @@ static void launch_script(const char *setup_script, const char *ifname,
>  static void tap_send(void *opaque);
>  static void tap_writable(void *opaque);
>  
> -static char *tap_parse_script(const char *script_arg, const char *default_path)
> +static bool tap_is_explicit_no_script(const char *script_arg)
>  {
> -    g_autofree char *res = g_strdup(script_arg);
> +    if (!script_arg) {
> +        return false;
> +    }
>  
> -    if (!res) {
> -        res = get_relocated_path(default_path);
> +    if (script_arg[0] == '\0') {
> +        return true;
> +    }
> +
> +    if (strcmp(script_arg, "no") == 0) {
> +        return true;
>      }
>  
> -    if (res[0] == '\0' || strcmp(res, "no") == 0) {
> +    return false;
> +}
> +
> +static char *tap_parse_script(const char *script_arg, const char *default_path)
> +{
> +    if (tap_is_explicit_no_script(script_arg)) {
>          return NULL;
>      }
>  
> -    return g_steal_pointer(&res);
> +    if (!script_arg) {
> +        return get_relocated_path(default_path);
> +    }
> +
> +    return g_strdup(script_arg);
>  }
>  
>  static void tap_update_fd_handler(TAPState *s)

Before the patch, we default @script_arg to get_relocated_path(default)
first, then check for "" or "no".  After the patch, we do it the other
way round.  Works, because get_relocated_path() never returns "" or
"no".  Worth mentioning in the commit message?

The new order feels slightly clearer to me.

Reviewed-by: Markus Armbruster <armbru@redhat.com>