[augeas-devel] [PATCH] Basic support for case insensitive square lens

[David Lutterkort]

On Tue, 2011-02-08 at 23:15 -0500, Francis Giraldeau wrote:
> Square lens must match open and close tag. In some situations, for instance
> httpd, open and close tags case is allowed to be different. If the square
> regexp is case insensitive, we allow case mismatch between open and close tag.
> The default case for the close tag is the one from the key in the create
> function, and in the put function, the original case is preserved.
> 
> As test case shows, some serious errors occurs when the lens regexp is set to
> nocase. Here is the error message while running pass_square.aug:
> 
> lt-augparse: put.c:236: split_concat: Assertion `regs.start[reg] != -1' failed.

I need to look into this - not quite clear to me what's missing there.

> ---
>  src/get.c                     |   55 +++++++++++++++++++++++++++++++++++++---
>  src/put.c                     |    7 +----
>  src/regexp.c                  |    3 +-
>  tests/modules/pass_square.aug |   36 ++++++++++++++++++++++++++
>  4 files changed, 89 insertions(+), 12 deletions(-)
> 
> diff --git a/src/get.c b/src/get.c
> index dbab6fa..49f7b78 100644
> --- a/src/get.c
> +++ b/src/get.c
> @@ -313,6 +313,37 @@ static int match(struct state *state, struct lens *lens,
>      return count;
>  }
>  
> +/* compare two strings in a case insensitive manner
> + * returns 0 if string matches, -1 otherwise
> + */
> +static int strcmp_nocase(char *s1, char *s2) {
> +    char *s1_re;
> +    int count, s2_len;
> +    int result = -1;
> +
> +    s2_len = strlen(s2);
> +    s1_re = strdup(s1);
> +    if (s1_re == NULL)
> +        goto error_dup;
> +
> +    struct regexp *re = make_regexp(NULL, s1_re, true);
> +    if (re == NULL)
> +        goto error_re;
> +
> +    count = regexp_match(re, s2, s2_len, 0, NULL);
> +
> +    if (count == s2_len)
> +        result = 0;
> +
> +    unref(re, regexp);
> +    return result;
> +
> + error_re:
> +   FREE(s1_re);
> + error_dup:
> +    return result;
> +}

This isn't necessary - use the STRCASEEQ macro from internal.h.
Similarly, you shouldn't use strcmp directly, use STREQ (strcmp has an
interface that is meant to hurt people)

Also, if you need to turn a string into a regexp that matches that
string, you need to use make_regexp_literal, since various characters
need to be escaped.

> diff --git a/src/put.c b/src/put.c
> index 32618c4..66030bb 100644
> --- a/src/put.c
> +++ b/src/put.c
> @@ -468,12 +468,7 @@ static void put_del(ATTRIBUTE_UNUSED struct lens *lens, struct state *state) {
>      assert(lens->tag == L_DEL);
>      assert(state->skel != NULL);
>      assert(state->skel->tag == L_DEL);
> -    if (lens->string != NULL) {
>      fprintf(state->out, "%s", state->skel->text);
> -    } else {
> -    /* L_DEL with NULL string: replicate the current key */
> -        fprintf(state->out, "%s", state->key);
> -    }
>  }

What's happening here ? Why don't we special case the square lens
anymore ?
 
>  static void put_union(struct lens *lens, struct state *state) {
> @@ -662,7 +657,7 @@ static void create_subtree(struct lens *lens, struct state *state) {
>  static void create_del(struct lens *lens, struct state *state) {
>      assert(lens->tag == L_DEL);
>      if (lens->string != NULL) {
> -    print_escaped_chars(state->out, lens->string->str);
> +        print_escaped_chars(state->out, lens->string->str);

White space cleanup should really go into a separate patch; makes it
easier to see what has changed.

> diff --git a/src/regexp.c b/src/regexp.c
> index 14020af..ca9f7a5 100644
> --- a/src/regexp.c
> +++ b/src/regexp.c
> @@ -117,7 +117,8 @@ struct regexp *make_regexp(struct info *info, char *pat, int nocase) {
>      struct regexp *regexp;
>  
>      make_ref(regexp);
> -    regexp->info = ref(info);
> +    if (info != NULL)
> +        regexp->info = ref(info);

Don't do that - all kinds of things can crash and burn when info is
NULL; pretty much any error routine assumes that info is around and can
be used to indicate to the user where an error comes from.

David