Re: [Nbd] [PATCH 2/4] nbd-server: fix parse_cfile() to not modify any global variables

[Wouter Verhelst <w@...112...>]

Hi Tuomas,

On Thu, Jan 03, 2013 at 11:20:53PM +0200, Tuomas Jorma Juhani Räsänen wrote:
> This change allows parse_cfile() to be called without any side-effects
> and passes the decision of what to do with parsed results to the
> caller.

Right, that's the big one :-)

[...]
> -	if(i==1 && have_global) {
> +	if(i==1 && genconf) {
>  		g_set_error(e, errdomain, CFILE_NO_EXPORTS, "The config file does not specify any exports");
>  	}
> +
> +        if (genconf) {
> +                /* Return the updated generic configuration through the
> +                 * pointer parameter. */
> +                memcpy(genconf, &genconftmp, sizeof(struct generic_conf));
> +        } else {
> +                /* The caller is not interested in the generic
> +                 * configuration, discard everything we parsed. */
> +                g_free(genconftmp.user);
> +                g_free(genconftmp.group);
> +                g_free(genconftmp.modernaddr);
> +                g_free(genconftmp.modernport);
> +        }

This seems wrong.

Maybe the "have_global" parameter was misnamed, but it's meant to be set
to FALSE when we're parsing a config file from an "includedir"
statement. That is, it calls itself recursively, and thus can be called
in two ways:
- Either it's called on the main config file,
  $sysconfdir/nbd-server/config, which is _required_ to have a section
  [generic]
- Or it's called on a config file _snippet_ from the directory specified
  in includedir; these are supposed to only define one export, and
  _cannot_ have a section [generic].

In other words, if you have something to throw away there, that would
mean you did something wrong.

-- 
Copyshops should do vouchers. So that next time some bureaucracy requires you
to mail a form in triplicate, you can mail it just once, add a voucher, and
save on postage.