[augeas-devel] [PATCH 2 of 3] Properly typecheck the '?' operator
David Lutterkort
lutter at redhat.com
Tue Sep 2 23:08:56 UTC 2008
10 files changed, 85 insertions(+), 30 deletions(-)
lenses/interfaces.aug | 6 ++----
lenses/sudoers.aug | 9 +++++----
lenses/tests/test_fstab.aug | 7 ++++---
lenses/tests/test_interfaces.aug | 7 ++++---
src/get.c | 8 ++++----
src/lens.c | 22 +++++++++++++++++++++-
src/lens.h | 2 +-
src/put.c | 28 ++++++++++++++++++----------
tests/modules/fail_del_maybe.aug | 7 +++++++
tests/modules/pass_store_maybe.aug | 19 +++++++++++++++++++
# HG changeset patch
# User David Lutterkort <dlutter at redhat.com>
# Date 1220396817 25200
# Node ID 359034962e09d7f86154f3f7225ef8091ed503c8
# Parent 495e0d2486712b7db2edb56819cd4b9a6f74ea78
Properly typecheck the '?' operator
The '?' operator was completely missing a typecheck for the atype. The
check must make sure that neither the ctype nor the atype match the empty
word.
To allow constructs like (store re)?, we allow the atype to match the empty
word, as long as the lens inside the '?' consumes the value of the current
node, since that also tells as whether to use the inner lens or not.
Various existing lenses failed to typecheck after this change and have been
adapted; this also fixes a bug in the Fstab.lns where existing whitespace
was being replaced with a default tab. The Interfaces.lns needed a fairly
invasive change, and as a consequence produces way more (too many?)
anonymous tree nodes.
Added tests to verify the typechecking of '?'
diff -r 495e0d248671 -r 359034962e09 lenses/interfaces.aug
--- a/lenses/interfaces.aug Tue Sep 02 15:58:51 2008 -0700
+++ b/lenses/interfaces.aug Tue Sep 02 16:06:57 2008 -0700
@@ -65,8 +65,7 @@
*************************************************************************)
let mapping = [ stanza_id "mapping"
- . ( eol | ((eol . empty*)? . comment+) )
- . empty*
+ . (comment|empty)+
. stanza_option
. (stanza_option|comment|empty)* ]
@@ -77,8 +76,7 @@
let iface = [ stanza_id "iface"
. stanza_param "family"
. stanza_param "method"
- . ( eol | ((eol . empty*)? . comment+) )
- . empty*
+ . (comment|empty)+
. ( stanza_option . (stanza_option|comment|empty)* )? ]
(************************************************************************
diff -r 495e0d248671 -r 359034962e09 lenses/sudoers.aug
--- a/lenses/sudoers.aug Tue Sep 02 15:58:51 2008 -0700
+++ b/lenses/sudoers.aug Tue Sep 02 16:06:57 2008 -0700
@@ -32,9 +32,10 @@
(* Define separators *)
let sep_spc = del /[ \t]+/ " "
let sep_cont = del /([ \t]+|[ \t]*\\\\\n[ \t]*)/ " "
-let sep_com = sep_cont? . Util.del_str "," . sep_cont?
-let sep_eq = sep_cont? . Util.del_str "=" . sep_cont?
-let sep_col = sep_cont? . Util.del_str ":" . sep_cont?
+let sep_cont_opt = del /([ \t]*|[ \t]*\\\\\n[ \t]*)/ " "
+let sep_com = sep_cont_opt . Util.del_str "," . sep_cont_opt
+let sep_eq = sep_cont_opt . Util.del_str "=" . sep_cont_opt
+let sep_col = sep_cont_opt . Util.del_str ":" . sep_cont_opt
(* Define fields *)
let sto_to_com_cmnd = store /([^,=:#() \t\n\\\\][^,=:#()\n\\\\]*[^,=:#() \t\n\\\\])|[^,=:#() \t\n\\\\]/
@@ -151,7 +152,7 @@
* Runas_Spec ::= '(' Runas_List ')'
*************************************************************************)
let runas_spec = Util.del_str "(" . alias_list "runas_user" sto_to_com
- . Util.del_str ")" . sep_cont?
+ . Util.del_str ")" . sep_cont_opt
(************************************************************************
* Tag_Spec ::= ('NOPASSWD:' | 'PASSWD:' | 'NOEXEC:' | 'EXEC:' |
diff -r 495e0d248671 -r 359034962e09 lenses/tests/test_fstab.aug
--- a/lenses/tests/test_fstab.aug Tue Sep 02 15:58:51 2008 -0700
+++ b/lenses/tests/test_fstab.aug Tue Sep 02 16:06:57 2008 -0700
@@ -13,7 +13,9 @@
let trailing_ws = "/dev/vg00/lv00\t /\t ext3\t defaults 1 1 \t\n"
- let no_passno = "LABEL=/boot\t /boot\t ext3\t defaults 1 \t\n"
+ let gen_no_passno(passno:string) =
+ "LABEL=/boot\t /boot\t ext3\t defaults 1" . passno . " \t\n"
+ let no_passno = gen_no_passno ""
let no_passno_tree =
{ "1"
@@ -51,8 +53,7 @@
test Fstab.lns get no_passno = no_passno_tree
- test Fstab.lns put no_passno after set "1/passno" "1" =
- "LABEL=/boot\t /boot\t ext3\t defaults\t1 1 \t\n"
+ test Fstab.lns put no_passno after set "1/passno" "1" = gen_no_passno " 1"
test Fstab.lns get no_dump = no_dump_tree
diff -r 495e0d248671 -r 359034962e09 lenses/tests/test_interfaces.aug
--- a/lenses/tests/test_interfaces.aug Tue Sep 02 15:58:51 2008 -0700
+++ b/lenses/tests/test_interfaces.aug Tue Sep 02 16:06:57 2008 -0700
@@ -47,7 +47,7 @@
{ "allow-hotplug" = "eth1" }
{ "iface" = "lo"
{ "family" = "inet"}
- { "method" = "loopback"} {} }
+ { "method" = "loopback"} {} {} }
{ "mapping" = "eth0"
{ "#comment" = "Home sweet home" }
{ "script" = "/usr/local/sbin/map-scheme"}
@@ -57,7 +57,7 @@
{ "iface" = "eth0-home"
{ "family" = "inet"}
{ "method" = "static"}
- {}
+ {} {}
{ "address" = "192.168.1.1" }
{ "netmask" = "255.255.255.0" }
{ "#comment" = "up flush-mail" }
@@ -65,7 +65,7 @@
{ "iface" = "eth0-work"
{ "family" = "inet"}
{ "method" = "dhcp"}
- {} }
+ {} {} }
{ "auto"
{ "1" = "eth1" } }
{ "iface" = "eth1"
@@ -74,6 +74,7 @@
{ "#comment" = "This is easy" }
{} }
{ "mapping" = "eth1"
+ {}
{ "#comment" = "I like mapping ..." }
{ "#comment" = "... and I like comments" }
{}
diff -r 495e0d248671 -r 359034962e09 src/get.c
--- a/src/get.c Tue Sep 02 15:58:51 2008 -0700
+++ b/src/get.c Tue Sep 02 16:06:57 2008 -0700
@@ -571,12 +571,12 @@
struct dict **dict) {
assert(lens->tag == L_MAYBE);
- struct skel *skel = make_skel(lens);
+ struct skel *skel = NULL;
if (applies(lens->child, state->split)) {
- struct skel *sk;
- sk = parse_lens(lens->child, state, dict);
- list_append(skel->skels, sk);
+ skel = parse_lens(lens->child, state, dict);
}
+ if (skel == NULL)
+ skel = make_skel(lens);
return skel;
}
diff -r 495e0d248671 -r 359034962e09 src/lens.c
--- a/src/lens.c Tue Sep 02 15:58:51 2008 -0700
+++ b/src/lens.c Tue Sep 02 16:06:57 2008 -0700
@@ -479,8 +479,28 @@
"illegal optional expression: /%s/ matches the empty word",
l->ctype->pattern->str);
}
+ fa_free(fa);
+
+ /* Typecheck the put direction; the check passes if
+ (1) the atype does not match the empty string, because we can tell
+ from looking at tree nodes whether L should be applied or not
+ (2) L handles a value; with that, we know whether to apply L or not
+ depending on whether the current node has a non NULL value or not
+ */
+ if (exn == NULL && ! l->value) {
+ fa = regexp_to_fa(l->atype);
+ if (fa == NULL)
+ return make_exn_value(ref(info),
+ "Internal error: regexp_to_fa failed");
+
+ eps = fa_make_basic(FA_EPSILON);
+ if (fa_contains(eps, fa)) {
+ exn = make_exn_value(ref(info),
+ "optional expression matches the empty tree");
+ }
+ fa_free(fa);
+ }
fa_free(eps);
- fa_free(fa);
return exn;
}
diff -r 495e0d248671 -r 359034962e09 src/lens.h
--- a/src/lens.h Tue Sep 02 15:58:51 2008 -0700
+++ b/src/lens.h Tue Sep 02 16:06:57 2008 -0700
@@ -89,7 +89,7 @@
enum lens_tag tag;
union {
char *text; /* L_DEL */
- struct skel *skels; /* L_CONCAT, L_STAR, L_MAYBE */
+ struct skel *skels; /* L_CONCAT, L_STAR */
};
/* Also tag == L_SUBTREE, with no data in the union */
};
diff -r 495e0d248671 -r 359034962e09 src/put.c
--- a/src/put.c Tue Sep 02 15:58:51 2008 -0700
+++ b/src/put.c Tue Sep 02 16:06:57 2008 -0700
@@ -257,15 +257,21 @@
}
/* Check if LENS applies to the current split in STATE */
-static int applies(struct lens *lens, struct split *split) {
+static int applies(struct lens *lens, struct state *state) {
int count;
+ struct split *split = state->split;
+
count = regexp_match(lens->atype, split->labels, split->end,
split->start, NULL);
if (count == -2) {
FIXME("Match failed - produce better error");
abort();
}
- return (count == split->end - split->start);
+ if (count != split->end - split->start)
+ return 0;
+ if (count == 0 && lens->value)
+ return state->value != NULL;
+ return 1;
}
/* Print TEXT to OUT, translating common escapes like \n */
@@ -360,8 +366,9 @@
break;
case L_SUBTREE:
return skel->tag == L_SUBTREE;
+ case L_MAYBE:
+ return skel->tag == L_MAYBE || skel_instance_of(lens->child, skel);
case L_STAR:
- case L_MAYBE:
if (skel->tag != lens->tag)
return 0;
if (lens->tag == L_MAYBE &&
@@ -427,7 +434,7 @@
for (int i=0; i < lens->nchildren; i++) {
struct lens *l = lens->children[i];
- if (applies(l, state->split)) {
+ if (applies(l, state)) {
if (skel_instance_of(l, state->skel))
put_lens(l, state);
else
@@ -491,12 +498,13 @@
static void put_quant_maybe(struct lens *lens, struct state *state) {
assert(lens->tag == L_MAYBE);
+ struct lens *child = lens->child;
- if (applies(lens->child, state->split)) {
- if (skel_instance_of(lens->child, state->skel))
- put_lens(lens->child, state);
+ if (applies(child, state)) {
+ if (skel_instance_of(child, state->skel))
+ put_lens(child, state);
else
- create_lens(lens->child, state);
+ create_lens(child, state);
}
}
@@ -574,7 +582,7 @@
assert(lens->tag == L_UNION);
for (int i=0; i < lens->nchildren; i++) {
- if (applies(lens->children[i], state->split)) {
+ if (applies(lens->children[i], state)) {
create_lens(lens->children[i], state);
return;
}
@@ -623,7 +631,7 @@
static void create_quant_maybe(struct lens *lens, struct state *state) {
assert(lens->tag == L_MAYBE);
- if (applies(lens->child, state->split)) {
+ if (applies(lens->child, state)) {
create_lens(lens->child, state);
}
}
diff -r 495e0d248671 -r 359034962e09 tests/modules/fail_del_maybe.aug
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/modules/fail_del_maybe.aug Tue Sep 02 16:06:57 2008 -0700
@@ -0,0 +1,7 @@
+(* The construct (del RE STR)? must raise an error from the typechecker *)
+(* since there's no way for the put of '?' to determine whether the del *)
+(* should be used or not - that decision is made by looking at the tree *)
+(* alone. *)
+module Fail_del_maybe =
+
+let indent = (del /[ \t]+/ " ")?
diff -r 495e0d248671 -r 359034962e09 tests/modules/pass_store_maybe.aug
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/modules/pass_store_maybe.aug Tue Sep 02 16:06:57 2008 -0700
@@ -0,0 +1,19 @@
+(* Test that the '?' operator behaves properly when it contains only *)
+(* a store. For this to work, the put for '?' has to take into account *)
+(* whether the current tree node has a value associated with it or not *)
+module Pass_store_maybe =
+
+ let lns = [ key /[a-z]+/ . del / +/ " " . (store /[0-9]+/) ? ]
+
+ test lns put "key " after rm "noop" = "key "
+ test lns put "key " after set "key" "42" = "key 42"
+ test lns put "key 42" after set "key" "13" = "key 13"
+
+ (* Convoluted way to make the value for "key" NULL *)
+ test lns put "key 42" after insa "key" "key"; rm "key[1]" = "key "
+
+ (* Check that we correctly restore the DEL if we choose to go into the *)
+ (* '?' operator instead of doing a create and using the "--" default *)
+ let ds = [ key /[a-z]+/ . (del /[ -]*/ "--" . store /[0-9]+/) ? ]
+
+ test ds put "key 0" after rm "noop" = "key 0"
More information about the augeas-devel
mailing list