Re: [RFC] Support for const char and strings in stabs reader

[Joel Brobecker <brobecker@adacore.com>]

> 2010-03-16  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	* stabsread.c (define_symbol): Add support for char
> 	and string constants.

As a general comment, the formatting of the section supporting strings
is not consistent. It looks like it's because you have a mixture of
spaces and tabs...

> +	case 's':
> +	  {
> +            struct type *range_type;
> +	    char quote = *p++;
> +            char *startp = p;
> +            gdb_byte *string_value;

Can you add an empty line after your local variable declarations?

> +            if (quote != '\'' && quote != '"')
> +              {
> +                SYMBOL_CLASS (sym) = LOC_CONST;
> +                SYMBOL_TYPE (sym) = error_type (&p, objfile);
> +                SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
> +                add_symbol_to_list (sym, &file_symbols);
> +                return sym;
> +              }
> +            /* Find matching quote, rejecting escaped quotes.  */
> +            while (*p && *p != quote)
> +              {
> +                if (*p == '\\')
> +                  p++;
> +                if (*p) 
> +                  p++;
> +              }

What if you don't find the matching quote?  I think we should reject
the stabs the same way you do when you don't find an opening quote.

> +            *p = '\0';

Outch. I see that you restore the stabs later on, but I am still
concerned by this, particularly if any of the functions you use end up
raising an exception...

It looks like you do this mostly to be able to get the length of
the string itself.  But that's easy to obtain, thanks to the loop
that searches the ending quote character.

> +	    strcpy ((char *)string_value, startp);
> +            *p = quote;

One question I asked myself, and got confused over, is: What happens
with escaped quotes. For instance, let's imagine that we have the
following string: 'hello \' there'.  I think that the string value
that you want to record is <<hello ' there>>, not <<hello \' there>>.
In other word, the symbol value should not contain the backslash, right?
Does it look like your current implementation will in fact contain it?

> +            range_type = create_range_type (NULL,
> +                           objfile_type (objfile)->builtin_int,
> +                           0, strlen(startp));

The indentation is wrong, it should be:

            range_type =
              create_range_type (NULL,
                                 objfile_type (objfile)->builtin_int,
                                 0, strlen(startp));

> +	   gdb_test "p /x constchar" " = 0x46" "char constant"
> +        gdb_test "p constString1" " = \"String1\"" "String constant 1"
> +        gdb_test "p constString2" " = \"String2\"" "String constant 2"
> +        gdb_test "p constString3" " = \"String3 with embedded quote ' in
> the middle\"" "String constant 3"

Your last test seems to contradict me on the escape character, but
how does this end up working?

-- 
Joel