All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andries.Brouwer@cwi.nl
To: Andries.Brouwer@cwi.nl, viro@math.psu.edu
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	torvalds@transmeta.com
Subject: Re: [RFC][PATCH] parser for mount options
Date: Tue, 7 Aug 2001 23:52:49 GMT	[thread overview]
Message-ID: <200108072352.XAA25661@vlet.cwi.nl> (raw)

> it breeds global variables for no good reason

Globals? What globals?

> Switch that will keep growing, BTW.
> would turn into complete mess two years down the road.

But this was written five and a half years ago, and I think it
still suffices.

> There are two different tasks - one of them is to decide which option we
> are dealing with and another - decode and act upon it.  Mixing parsing
> and data conversion in that kind of situations is a Bad Thing(tm).

Possibly. I like the option parsing for each filesystem:

	parse_mount_options((char *) data, SIZE(opts), opts);

and this does parse_and_assign. You do

	while (more_tokens) {
		t = type_of_next_token();
		switch (t) {
		case ...
		}
	}

where the type_of_next_token() does the parsing, and the switch
does the assigning. Much more code. Much uglier - but tastes differ.
The reason that I call it uglier is that you have the same, or
nearly the same code for each filesystem. But then discrepancies arise,
and things are not treated uniformly across filesystems. A single
parser and assigner forces uniformity.
You have a coherency problem. In

+enum { Opt_mode, ...};
+                               
+static match_table_t tokens = {
+       {Opt_mode, "mode=%o"},
...
+                       case Opt_mode:
+                               mode = match_octal(args);
+                               break;

the %o must correspond to the match_octal().
But that is unfortunate duplication.
That same code is

	{ "mode", OPT_INT_8, 0, &mode},

for me. Not only much more compact, but no coherency problem either.
Of course one might write

	{ "mode", "%o", 0, &mode},

to save the reader the trouble of looking up what OPT_INT_8 means.

If you see strange warts in my parser it is mostly because
it was a patch without user-visible changes, so all existing
msdos option peculiarities had to be accommodated.
Once such code is in place one needs a very good reason to
invent option syntax not covered by it.

Andries

             reply	other threads:[~2001-08-07 23:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-08-07 23:52 Andries.Brouwer [this message]
2001-08-08  1:14 ` [RFC][PATCH] parser for mount options Alexander Viro
  -- strict thread matches above, loose matches on Subject: below --
2001-08-08 11:38 Andries.Brouwer
2001-08-07 21:51 Andries.Brouwer
2001-08-07 22:02 ` Andreas Dilger
2001-08-07 22:40 ` Alexander Viro
2001-08-07 17:02 Alexander Viro
2001-08-07 18:06 ` Daniel Phillips
2001-08-07 18:12   ` Christoph Hellwig
2001-08-07 21:54 ` Pavel Machek
2001-08-07 22:48   ` Alexander Viro
2001-08-09  6:20 ` Albert D. Cahalan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200108072352.XAA25661@vlet.cwi.nl \
    --to=andries.brouwer@cwi.nl \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@transmeta.com \
    --cc=viro@math.psu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.