On Wed, Oct 28, 2020 at 07:52:57AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Now we have the config file parsing hooked up and feeding in
> parameters to mkfs, wire the parameters up to the existing CLI
> option parsing functions. THis gives the config file exactly the
> same capabilities and constraints as the command line option
> specification.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> mkfs/xfs_mkfs.c | 94 +++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 79 insertions(+), 15 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 829f0383b602..635a36f393b7 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -143,6 +143,12 @@ enum {
> * name MANDATORY
> * Name is a single char, e.g., for '-d file', name is 'd'.
> *
> + * ini_section MANDATORY
> + * This field is required to connect each opt_params (that is to say, each
> + * option class) to a section in the config file. The only option class this
> + * is not required for is the config file specification class itself.
> + * The section name is a string, not longer than MAX_INI_NAME_LEN.
> + *
> * subopts MANDATORY
> * Subopts is a list of strings naming suboptions. In the example above,
> * it would contain "file". The last entry of this list has to be NULL.
> @@ -201,6 +207,8 @@ enum {
> */
> struct opt_params {
> const char name;
> +#define MAX_INI_NAME_LEN 32
> + const char ini_section[MAX_INI_NAME_LEN];
> const char *subopts[MAX_SUBOPTS];
>
> struct subopt_param {
> @@ -228,6 +236,7 @@ static struct opt_params sopts;
>
> static struct opt_params bopts = {
> .name = 'b',
> + .ini_section = "block",
> .subopts = {
> [B_SIZE] = "size",
> },
> @@ -267,6 +276,7 @@ static struct opt_params copts = {
>
> static struct opt_params dopts = {
> .name = 'd',
> + .ini_section = "data",
> .subopts = {
> [D_AGCOUNT] = "agcount",
> [D_FILE] = "file",
> @@ -411,6 +421,7 @@ static struct opt_params dopts = {
>
> static struct opt_params iopts = {
> .name = 'i',
> + .ini_section = "inode",
> .subopts = {
> [I_ALIGN] = "align",
> [I_MAXPCT] = "maxpct",
> @@ -472,6 +483,7 @@ static struct opt_params iopts = {
>
> static struct opt_params lopts = {
> .name = 'l',
> + .ini_section = "log",
> .subopts = {
> [L_AGNUM] = "agnum",
> [L_INTERNAL] = "internal",
> @@ -571,6 +583,7 @@ static struct opt_params lopts = {
>
> static struct opt_params nopts = {
> .name = 'n',
> + .ini_section = "naming",
> .subopts = {
> [N_SIZE] = "size",
> [N_VERSION] = "version",
> @@ -602,6 +615,7 @@ static struct opt_params nopts = {
>
> static struct opt_params ropts = {
> .name = 'r',
> + .ini_section = "realtime",
> .subopts = {
> [R_EXTSIZE] = "extsize",
> [R_SIZE] = "size",
> @@ -652,6 +666,7 @@ static struct opt_params ropts = {
>
> static struct opt_params sopts = {
> .name = 's',
> + .ini_section = "sector",
> .subopts = {
> [S_SIZE] = "size",
> [S_SECTSIZE] = "sectsize",
> @@ -682,6 +697,7 @@ static struct opt_params sopts = {
>
> static struct opt_params mopts = {
> .name = 'm',
> + .ini_section = "metadata",
> .subopts = {
> [M_CRC] = "crc",
> [M_FINOBT] = "finobt",
> @@ -982,6 +998,17 @@ unknown(
> usage();
> }
>
> +static void
> +invalid_cfgfile_opt(
> + const char *filename,
> + const char *section,
> + const char *name,
> + const char *value)
> +{
> + fprintf(stderr, _("%s: invalid config file option: [%s]:%s=%s\n"),
> + filename, section, name, value);
Silly nit: space after the last colon.
"/etc/xfs.conf: invalid config file option: [metadata]: crc=0"
(A pity libinih doesn't tell you the line number...)
Also, does this really need a separate function since it's only called
from one place?
> +}
> +
> static void
> check_device_type(
> const char *name,
> @@ -1696,23 +1723,22 @@ sector_opts_parser(
> }
>
> static struct subopts {
> - char opt;
> struct opt_params *opts;
> int (*parser)(struct opt_params *opts,
> int subopt,
> const char *value,
> struct cli_params *cli);
> } subopt_tab[] = {
> - { 'b', &bopts, block_opts_parser },
> - { 'c', &copts, cfgfile_opts_parser },
> - { 'd', &dopts, data_opts_parser },
> - { 'i', &iopts, inode_opts_parser },
> - { 'l', &lopts, log_opts_parser },
> - { 'm', &mopts, meta_opts_parser },
> - { 'n', &nopts, naming_opts_parser },
> - { 'r', &ropts, rtdev_opts_parser },
> - { 's', &sopts, sector_opts_parser },
> - { '\0', NULL, NULL },
> + { &bopts, block_opts_parser },
> + { &copts, cfgfile_opts_parser },
> + { &dopts, data_opts_parser },
> + { &iopts, inode_opts_parser },
> + { &lopts, log_opts_parser },
> + { &mopts, meta_opts_parser },
> + { &nopts, naming_opts_parser },
> + { &ropts, rtdev_opts_parser },
> + { &sopts, sector_opts_parser },
> + { NULL, NULL },
> };
>
> static void
> @@ -1726,7 +1752,7 @@ parse_subopts(
> int ret = 0;
>
> while (sop->opts) {
> - if (sop->opt == opt)
> + if (opt && sop->opts->name == opt)
When does parse_subopts get passed a opt==0? AFAICT the only caller is
the switch statement under getopt, which constrains c to one of
'bdilmnrsc'.
--D
> break;
> sop++;
> }
> @@ -1749,6 +1775,45 @@ parse_subopts(
> }
> }
>
> +static bool
> +parse_cfgopt(
> + const char *section,
> + const char *name,
> + const char *value,
> + struct cli_params *cli)
> +{
> + struct subopts *sop = &subopt_tab[0];
> + char **subopts;
> + int ret = 0;
> + int i;
> +
> + while (sop->opts) {
> + if (sop->opts->ini_section[0] != '\0' &&
> + strcasecmp(section, sop->opts->ini_section) == 0)
> + break;
> + sop++;
> + }
> +
> + /* Config files with unknown sections get caught here. */
> + if (!sop->opts)
> + goto invalid_opt;
> +
> + subopts = (char **)sop->opts->subopts;
> + for (i = 0; i < MAX_SUBOPTS; i++) {
> + if (!subopts[i])
> + break;
> + if (strcasecmp(name, subopts[i]) == 0) {
> + ret = (sop->parser)(sop->opts, i, value, cli);
> + if (ret)
> + goto invalid_opt;
> + return true;
> + }
> + }
> +invalid_opt:
> + invalid_cfgfile_opt(cli->cfgfile, section, name, value);
> + return false;
> +}
> +
> static void
> validate_sectorsize(
> struct mkfs_params *cfg,
> @@ -3630,9 +3695,8 @@ cfgfile_parse_ini(
> {
> struct cli_params *cli = user;
>
> - fprintf(stderr, "Ini debug: file %s, section %s, name %s, value %s\n",
> - cli->cfgfile, section, name, value);
> -
> + if (!parse_cfgopt(section, name, value, cli))
> + return 0;
> return 1;
> }
>
> --
> 2.28.0
>
