[Cbe-oss-dev] [patch 16/18] petitboot: Add system helper routines

[Jeremy Kerr]

> -ui_common_objs = ui/common/discover-client.o
> +ui_common_objs = ui/common/discover-client.o ui/common/pb-system.o

I've kinda moved away from the pb- prefix on the object files, how about 
just 'system' or 'system-helpers' ?

> +int pb_run_system(const char *cmd)

I'm a bit weary of this method of executing external programs, as we can 
end up in trouble with shell metacharachters. Can we do this with an 
array of char * instead?

> +	if (result) {
> +		cmd = talloc_asprintf(NULL, "%s -f%s%s%s%s%s%s %s",
> +		pb_system_apps.kexec,
> +		(l_initrd ? " --initrd='" : ""),
> +		(l_initrd ? l_initrd : ""),
> +		(l_initrd ? "'" : ""),
> +		(args ? " --append='" : ""),
> +		(args ? args : ""),
> +		(args ? "'" : ""),
> +		l_image);
> +
> +		pb_log("%s: '%s'\n", __func__, cmd);
> +
> +		result = pb_run_system(cmd);
> +		talloc_free(cmd);
> +	}

.. and it means we don't have to do this kind of sprintf to construct a 
command line.


> +	static const char s_file[] = "file://";
> +	static const char s_ftp[] = "ftp://";
> +	static const char s_http[] = "http://";
> +	static const char s_https[] = "https://";
> +	static const char s_nfs[] = "nfs://";
> +	static const char s_scp[] = "scp://";
> +	static const char s_tftp[] = "tftp://";

[snip]

> +
> +	if (!strncasecmp(s_url, s_file, sizeof(s_file) - 1)) {
> +		url->scheme = pb_url_file;
> +		p = s_url + sizeof(s_file) - 1;
> +		goto got_scheme;
> +	}
> +
> +	if (!strncasecmp(s_url, s_ftp, sizeof(s_ftp) - 1)) {
> +		url->scheme = pb_url_ftp;
> +		p = s_url + sizeof(s_ftp) - 1;
> +		goto got_scheme;
> +	}


static const struct {
	const char *scheme_str;
	enum pb_url_scheme scheme;
} schemes[] = {
	{ "tftp", pb_url_tftp },
	{ "file", pb_url_file },
    .... 
}

for (i = 0; i < ARRAY_SIZE(schemes); i++) {
	int len = strlen(schemes[i].scheme_str);

	if (strncasecmp(s_url, schemes[i].scheme_str, len)
		continue;

	url->scheme = schemes[i].scheme
	p = s_url + len;
}

We could even hook the load_*() functions into that schemes[] array, 
which would simplify pb_load_file()...

(or maybe do this in a separate common/url.c object? up to you.)

Cheers,

Jeremy