When I converted befs's option parsing to use the new lib/parser.c
functions, I had to read the functions (and the patch converting ext3)
in order to understand exactly how to use them. They are not that
complicated, but since I'd already read and (hopefully) understood the
functions, I figured I'd add a bit of documentation for others.
I am not the author of the functions I am attempting to document here,
so any mistakes are just that: mistakes on my part.
# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or
higher.
# This patch includes the following deltas:
# ChangeSet 1.1352 -> 1.1353
# lib/parser.c 1.2 -> 1.3
# include/linux/parser.h 1.1 -> 1.2
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/11/16 will@thalience.(none) 1.1353
# Add documentation and comments to lib/parser.c and
include/linux/parser.h
# --------------------------------------------
#
diff -Nru a/include/linux/parser.h b/include/linux/parser.h
--- a/include/linux/parser.h Sun Nov 16 03:02:39 2003
+++ b/include/linux/parser.h Sun Nov 16 03:02:39 2003
@@ -1,3 +1,14 @@
+/*
+ * linux/include/linux/parser.h
+ *
+ * Header for lib/parser.c
+ * Intended use of these functions is parsing filesystem argument
lists,
+ * but could potentially be used anywhere else that simple option=arg
+ * parsing is required.
+ */
+
+
+// associates an integer enumerator with a pattern string.
struct match_token {
int token;
char *pattern;
@@ -5,17 +16,18 @@
typedef struct match_token match_table_t[];
+// Maximum number of arguments that match_token will find in a pattern
enum {MAX_OPT_ARGS = 3};
+// Describe the location within a string of a substring
typedef struct {
char *from;
char *to;
} substring_t;
int match_token(char *s, match_table_t table, substring_t args[]);
-
-int match_int(substring_t *, int *result);
-int match_octal(substring_t *, int *result);
-int match_hex(substring_t *, int *result);
-void match_strcpy(char *, substring_t *);
-char *match_strdup(substring_t *);
+int match_int(substring_t *s, int *result);
+int match_octal(substring_t *s, int *result);
+int match_hex(substring_t *s, int *result);
+void match_strcpy(char *to, substring_t *s);
+char *match_strdup(substring_t *s);
diff -Nru a/lib/parser.c b/lib/parser.c
--- a/lib/parser.c Sun Nov 16 03:02:39 2003
+++ b/lib/parser.c Sun Nov 16 03:02:39 2003
@@ -11,6 +11,17 @@
#include <linux/slab.h>
#include <linux/string.h>
+/**
+ * match_one: - Determines if a string matches a simple pattern
+ * @s: the string to examine for presense of the pattern
+ * @p: the string containing the pattern
+ * @args: array of %MAX_OPT_ARGS &substring_t elements. Used to return
match
+ * locations.
+ *
+ * Description: Determines if the pattern @p is present in string @s.
Can only
+ * match extremely simple token=arg style patterns. If the pattern is
found,
+ * the location(s) of the arguments will be returned in the @args
array.
+ */
static int match_one(char *s, char *p, substring_t args[])
{
char *meta;
@@ -74,6 +85,20 @@
}
}
+/**
+ * match_token: - Find a token (and optional args) in a string
+ * @s: the string to examine for token/argument pairs
+ * @table: match_table_t describing the set of allowed option tokens
and the
+ * arguments that may be associated with them. Must be terminated with
a
+ * &struct match_token who's pattern is set to the NULL pointer.
+ * @args: array of %MAX_OPT_ARGS &substring_t elements. Used to return
match
+ * locations.
+ *
+ * Description: Detects which if any of a set of token strings has been
passed
+ * to it. Tokens can include up to MAX_OPT_ARGS instances of basic
c-style
+ * format identifiers which will be taken into account when matching
the
+ * tokens, and who's locations will be returned in the @args array.
+ */
int match_token(char *s, match_table_t table, substring_t args[])
{
struct match_token *p;
@@ -84,6 +109,16 @@
return p->token;
}
+/**
+ * match_number: scan a number in the given base from a substring_t
+ * @s: substring to be scanned
+ * @result: resulting integer on success
+ * @base: base to use when converting string
+ *
+ * Description: Given a &substring_t and a base, attempts to parse the
substring
+ * as a number in that base. On success, sets @result to the integer
represented
+ * by the string and returns 0. Returns either -ENOMEM or -EINVAL on
failure.
+ */
static int match_number(substring_t *s, int *result, int base)
{
char *endp;
@@ -103,27 +138,71 @@
return ret;
}
+/**
+ * match_int: - scan a decimal representation of an integer from a
substring_t
+ * @s: substring_t to be scanned
+ * @result: resulting integer on success
+ *
+ * Description: Attempts to parse the &substring_t @s as a decimal
integer. On
+ * success, sets @result to the integer represented by the string and
returns 0.
+ * Returns either -ENOMEM or -EINVAL on failure.
+ */
int match_int(substring_t *s, int *result)
{
return match_number(s, result, 0);
}
+/**
+ * match_octal: - scan an octal representation of an integer from a
substring_t
+ * @s: substring_t to be scanned
+ * @result: resulting integer on success
+ *
+ * Description: Attempts to parse the &substring_t @s as an octal
integer. On
+ * success, sets @result to the integer represented by the string and
returns
+ * 0. Returns either -ENOMEM or -EINVAL on failure.
+ */
int match_octal(substring_t *s, int *result)
{
return match_number(s, result, 8);
}
+/**
+ * match_hex: - scan a hex representation of an integer from a
substring_t
+ * @s: substring_t to be scanned
+ * @result: resulting integer on success
+ *
+ * Description: Attempts to parse the &substring_t @s as a hexadecimal
integer.
+ * On success, sets @result to the integer represented by the string
and
+ * returns 0. Returns either -ENOMEM or -EINVAL on failure.
+ */
int match_hex(substring_t *s, int *result)
{
return match_number(s, result, 16);
}
+/**
+ * match_strcpy: - copies the characters from a substring_t to a
c-string
+ * @to: c-string to copy characters to.
+ * @s: &substring_t to copy
+ *
+ * Description: Copies the set of characters represented by the given
+ * &substring_t @s to the c-style string @to. Caller guarantees that
@to is
+ * large enough to hold the characters of @s.
+ */
void match_strcpy(char *to, substring_t *s)
{
memcpy(to, s->from, s->to - s->from);
to[s->to - s->from] = '\0';
}
+/**
+ * match_strdup: - allocate a new c-string with the contents of a
substring_t
+ * @s: &substring_t to copy
+ *
+ * Description: Allocates and returns a c-string filled with the
contents of
+ * the &substring_t @s. The caller is responsible for freeing the
returned
+ * string with kfree().
+ */
char *match_strdup(substring_t *s)
{
char *p = kmalloc(s->to - s->from + 1, GFP_KERNEL);
--
Will Dyson
"Back off man, I'm a scientist!" -Dr. Peter Venkman
On Sun, Nov 16, 2003 at 03:16:03AM -0500, Will Dyson wrote:
> +// associates an integer enumerator with a pattern string.
Please no C++ comments.
> -int match_int(substring_t *, int *result);
> -int match_octal(substring_t *, int *result);
> -int match_hex(substring_t *, int *result);
> -void match_strcpy(char *, substring_t *);
> -char *match_strdup(substring_t *);
> +int match_int(substring_t *s, int *result);
> +int match_octal(substring_t *s, int *result);
> +int match_hex(substring_t *s, int *result);
> +void match_strcpy(char *to, substring_t *s);
> +char *match_strdup(substring_t *s);
What value does this "s" add? "result" is clearly useful documentation,
but "s" says "There is no good name for this variable"
> @@ -74,6 +85,20 @@
> }
> }
>
> +/**
> + * match_token: - Find a token (and optional args) in a string
> + * @s: the string to examine for token/argument pairs
> + * @table: match_table_t describing the set of allowed option tokens
> and the
> + * arguments that may be associated with them. Must be terminated with
> a
> + * &struct match_token who's pattern is set to the NULL pointer.
whose
> + * @args: array of %MAX_OPT_ARGS &substring_t elements. Used to return
> match
> + * locations.
> + *
> + * Description: Detects which if any of a set of token strings has been
> passed
> + * to it. Tokens can include up to MAX_OPT_ARGS instances of basic
> c-style
> + * format identifiers which will be taken into account when matching
> the
> + * tokens, and who's locations will be returned in the @args array.
ditto
> +/**
> + * match_strdup: - allocate a new c-string with the contents of a
Umm. We're writing in C. Just plain "string" is fine.
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
Matthew Wilcox wrote:
> On Sun, Nov 16, 2003 at 03:16:03AM -0500, Will Dyson wrote:
> > +// associates an integer enumerator with a pattern string.
>
> Please no C++ comments.
"//" comments have been in standard C since 1999.
For the sake of stylistic consistency by all means exclude them, but
please don't call them C++ now that they are standard in C.
Thanks :)
-- Jamie
On Sun, Nov 16, 2003 at 06:20:07PM +0000, Jamie Lokier wrote:
> Matthew Wilcox wrote:
> > On Sun, Nov 16, 2003 at 03:16:03AM -0500, Will Dyson wrote:
> > > +// associates an integer enumerator with a pattern string.
> >
> > Please no C++ comments.
>
> "//" comments have been in standard C since 1999.
>
> For the sake of stylistic consistency by all means exclude them, but
> please don't call them C++ now that they are standard in C.
Good luck with that. Note that syphilis kept the name of French disease
for many decades after it had become standard in all European countries,
even though it almost definitely did not originate in .fr in the first
place.
On Sun, 2003-11-16 at 11:09, Matthew Wilcox wrote:
> > -int match_int(substring_t *, int *result);
> > +int match_int(substring_t *s, int *result);
>
> What value does this "s" add? "result" is clearly useful documentation,
> but "s" says "There is no good name for this variable"
True, but that is what it is named in the function definition. I'll try
to think of a good name for these, but I guess a documentation patch is
not the place for that (since I'd want to change it in both declaration
and definition).
Your other comments are well-taken as well.
--
Will Dyson
"Back off man, I'm a scientist!" -Dr. Peter Venkman
On Sun, 2003-11-16 at 03:16, Will Dyson wrote:
> When I converted befs's option parsing to use the new lib/parser.c
> functions, I had to read the functions (and the patch converting ext3)
> in order to understand exactly how to use them. They are not that
> complicated, but since I'd already read and (hopefully) understood the
> functions, I figured I'd add a bit of documentation for others.
>
> I am not the author of the functions I am attempting to document here,
> so any mistakes are just that: mistakes on my part.
Here is take 2, incorporating Matthew Wilcox's suggestions and sent as
an attatchment to avoid word-wrap.
--
Will Dyson
"Back off man, I'm a scientist!" -Dr. Peter Venkman
On Sun, 2003-11-16 17:37:05 -0500, Will Dyson <[email protected]>
wrote in message <1069022225.19499.59.camel@thalience>:
> On Sun, 2003-11-16 at 03:16, Will Dyson wrote:
> -int match_token(char *s, match_table_t table, substring_t args[]);
> -
> +int match_token(char *, match_table_t table, substring_t args[]);
Dropping the blank line is okay, but I don't like dropping "s"
altogether:)
MfG, JBG
--
Jan-Benedict Glaw [email protected] . +49-172-7608481
"Eine Freie Meinung in einem Freien Kopf | Gegen Zensur | Gegen Krieg
fuer einen Freien Staat voll Freier B?rger" | im Internet! | im Irak!
ret = do_actions((curr | FREE_SPEECH) & ~(NEW_COPYRIGHT_LAW | DRM | TCPA));
On Mon, 2003-11-17 at 02:28, Jan-Benedict Glaw wrote:
> On Sun, 2003-11-16 17:37:05 -0500, Will Dyson <[email protected]>
> wrote in message <1069022225.19499.59.camel@thalience>:
> > On Sun, 2003-11-16 at 03:16, Will Dyson wrote:
>
> > -int match_token(char *s, match_table_t table, substring_t args[]);
> > -
> > +int match_token(char *, match_table_t table, substring_t args[]);
>
> Dropping the blank line is okay, but I don't like dropping "s"
> altogether:)
Well, I think it should be consistent. My original patch added "s" to
all of the prototypes, on the basis that that is the name given in the
function definition (and a partial misunderstanding of kernel-doc). But
it was pointed out that "s" is highly uninformative as an argument name,
and serves no documentation purpose. So I got rid of it, with the
intention of providing another patch which changes the argument names to
something better.
For what it's worth, however, I didn't realize that the original
match_token had the "s" when I re-diffed the patch earlier. And having
the blank line does make some sense, because match_token is different
from the others.
Got any ideas about how to name that argument in a way that is more
helpful to a developer looking to use the functions? I was thinking
"char *token" for match_token (because you must tokenize the argument
string before feeding each token to match_token) and "substring_t arg"
for the others.
Here is a(nother) rediff of the kernel-doc patch, changing no prototypes
at all. And also a follow-on that renames the arguments in the manner I
describe in the previous paragraph. Feel free to provide an alternate
renaming patch if you've got a better idea than "token" and "arg".
--
Will Dyson
"Back off man, I'm a scientist!" -Dr. Peter Venkman
On Mon, 17 Nov 2003 04:29:29 -0500 Will Dyson <[email protected]> wrote:
| On Mon, 2003-11-17 at 02:28, Jan-Benedict Glaw wrote:
| > On Sun, 2003-11-16 17:37:05 -0500, Will Dyson <[email protected]>
| > wrote in message <1069022225.19499.59.camel@thalience>:
| > > On Sun, 2003-11-16 at 03:16, Will Dyson wrote:
| >
| > > -int match_token(char *s, match_table_t table, substring_t args[]);
| > > -
| > > +int match_token(char *, match_table_t table, substring_t args[]);
| >
| > Dropping the blank line is okay, but I don't like dropping "s"
| > altogether:)
First, thanks for doing this since I never got around to it.
I like having the arg names in function prototypes, but they don't
have to be terribly descriptive IMO. Read the kernel-doc for
descriptions...
Consequently I don't find the arg-rename patch needed.
BTW, where did you find good references for creating kernel-doc?
...
| Got any ideas about how to name that argument in a way that is more
| helpful to a developer looking to use the functions? I was thinking
| "char *token" for match_token (because you must tokenize the argument
| string before feeding each token to match_token) and "substring_t arg"
| for the others.
|
| Here is a(nother) rediff of the kernel-doc patch, changing no prototypes
| at all. And also a follow-on that renames the arguments in the manner I
| describe in the previous paragraph. Feel free to provide an alternate
| renaming patch if you've got a better idea than "token" and "arg".
Evolution mangles in-line patches?? That's too bad.
Attachments are more difficult to review/reply to.
+++ b/include/linux/parser.h Mon Nov 17 04:02:55 2003
@@ -1,3 +1,14 @@
+/*
+ * linux/include/linux/parser.h
+ *
+ * Header for lib/parser.c
Don't need that last line. Kernel headers don't normally say things
like that, and it's #included by callers to parser as well as parser
itself. I.e., it's not only for lib/parser.c.
+++ b/lib/parser.c Mon Nov 17 04:02:55 2003
for match_token:
+ * Description: Detects which if any of a set of token strings has been passed
+ * to it. Tokens can include up to MAX_OPT_ARGS instances of basic c-style
+ * format identifiers which will be taken into account when matching the
+ * tokens, and whose locations will be returned in the @args array.
Use %MAX_OPT_ARGS consistently.
Don't need "c-style" at all IMO, or at least make it "C-style".
for match_strcpy:
+ * Description: Copies the set of characters represented by the given
+ * &substring_t @s to the c-style string @to. Caller guarantees that @to is
+ * large enough to hold the characters of @s.
s/c-style//
Thanks again.
--
~Randy
MOTD: Always include version info.
> BTW, where did you find good references for creating kernel-doc?
I too ask where?
> Evolution mangles in-line patches??
Seemingly yes by default.
> That's too bad.
> Attachments are more difficult to review/reply to.
The Ximian Evolution 1.2.2 here by default does style text "Normal"
i.e. aggressively line broken only. But the "Preformat" style works for
inline text patches such as my (: newly minted and beautiful but not yet
rejected for an explicit reason :) linux-scsi patches for making more
writable devices appear writable.
Pat LaVarre
P.S.
This paragraph here now is an example of Evolution's Preformat style of English with which we can let the line of text go on and on and on as English so very easily does when written by people who haven't yet learned to limit the length of a sentence, or the length of an email, helpfully.
On Mon, 2003-11-17 at 14:38, Pat LaVarre wrote:
> > BTW, where did you find good references for creating kernel-doc?
>
> I too ask where?
Documentation/kernel-doc-nano-HOWTO.txt was the best I found. It seems
reasonably complete, even though the name seems to imply there is a full
howto out there somewhere.
> > Evolution mangles in-line patches??
> The Ximian Evolution 1.2.2 here by default does style text "Normal"
> i.e. aggressively line broken only. But the "Preformat" style works for
> inline text patches such as my (: newly minted and beautiful but not yet
> rejected for an explicit reason :) linux-scsi patches for making more
> writable devices appear writable.
Thanks for the tip!
--
Will Dyson
"Back off man, I'm a scientist!" -Dr. Peter Venkman