Re: [PATCH] mini-os: netfront: Fix netfront init when no netmask or gateway are provided

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.09.2020 18:25, Costin Lupu wrote:
> Commit 1b8ed31f introduced support for reading netmask and gateway values
> from Xenstore. However it did not take care of the initial scenario when
> these values are not provided and that is what this patch tries to fix.
> 

Once again
Reported-by: Jan Beulich <jbeulich@xxxxxxxx>
or some such. You've made me (again) analyze breakage (still assuming
it is as suspected, since as said I had no data to actually prove my
suspicion, and it's not clear whether you've confirmed it really is,
or whether you've simply made the change "just in case"), so in one
way or another I'd like this to be expressed. I'm sorry for being a
little angry here, but after the first breakage I think you should
really have looked out for everything else going smoothly.

> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> ---
>  netfront.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/netfront.c b/netfront.c
> index 205484b..1a30945 100644
> --- a/netfront.c
> +++ b/netfront.c
> @@ -376,9 +376,9 @@ out:
>       }
>      if (ip)
>          *ip = strdup(ldev->ip);
> -    if (mask)
> +    if (mask && ldev->mask)
>          *mask = strdup(ldev->mask);
> -    if (gw)
> +    if (gw && ldev->gw)
>          *gw = strdup(ldev->gw);

So this introduces a (not written down afaict) requirement for the
caller to pre-initialize *mask and *gw, if passing non-NULL into
here. I'd have expected

    if (ip)
        *ip = strdup(ldev->ip);
    if (mask)
        *mask = ldev->mask ? strdup(ldev->mask) : NULL;
    if (gw)
        *gw = ldev->gw ? strdup(ldev->gw) : NULL;

(or something substantially similar) instead. Sorry for being picky.

As an aside I'm also wondering how in your new model one would
express address and gateway with the caller passing NULL just
for mask. Furthermore I think the conventional n.n.n.n/<width>
way of expressing a mask would also be useful to be recognized.

Jan