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

[Francis Giraldeau]

On Wed, 2011-02-16 at 17:02 -0800, David Lutterkort wrote:
> On Tue, 2011-02-08 at 23:15 -0500, Francis Giraldeau wrote:
> > 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.

Could it be that when case insensitive is set, the number of groups in
the regexp is modified?

> > +/* compare two strings in a case insensitive manner
> > + * returns 0 if string matches, -1 otherwise
> > + */
> > +static int strcmp_nocase(char *s1, char *s2) {
> 
> 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.

Thanks for the details. The function is removed in the new version of
the patch and the macro is used instead.

> > 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 ?

If the tags doesn't match in case, then to respect the GetPut function,
the right case must be put back. In the case of create only we copy the
key. This code was right when the both case were assumed to be the same.
 
> >  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.

Yeah... dirty patch. Will take care.

> > 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.

So right, it's like playing with fire. Fixed. In general, v2 patch is
very minimal. 

Cheer,

Francis