[RFA 1/4] Constify LEXPTR
Keith Seitz
keiths@redhat.com
Tue Oct 1 17:48:00 GMT 2013
Hi, Sergio,
On 09/30/2013 09:06 PM, Sergio Durigan Junior wrote:
> On Monday, September 30 2013, Keith Seitz wrote:
> I don't know how you're planning to commit that, but I'd vote for push
> the final result of applying all patches, because I don't see a strong
> reason to commit each patch separately in this specific case. WDYT?
> But IANAM.
Yeah, I would probably commit as one big one. That's how I usually test
on patchsets anyway. I just split them up to try and make them more
manageable for reviewers. [Notice I said, "try." I seem to seldom
succeed lately!]
[ChangeLog hasn't changed, so I'm just leaving it in this reply.]
>> ChangeLog
>> 2013-09-24 Keith Seitz <keiths@redhat.com>
>>
>> * c-exp.y (parse_number): Make first argument const.
>> Make a copy of the input to manipulate.
>> (c_parse_escape): Make first argument const.
>> Make local variable 'tokptr' const.
>> (parse_string_or_char): Make first two arguments const.
>> (macro_original_text): Make const.
>> (lex_one_token): Make local variable 'tokstart' const.
>> Likewise for local variables named 'p'.
>> Cast away const for struct stoken (temporary).
>> * c-lang.h (c_parse_escpae): Make first argument const.
>> * cli/cli-cmds.c (echo_command): Make local variable 'p'
>> const.
>> * cli/cli-setshow.c (do_set_command): Likewise for 'p' in
>> var_string case.
>> * f-exp.y (parse_number): Make first argument const.
>> (match_string_literal): Make local variable 'tokstart'
>> const.
>> (yylex): Make local variable 'p' const.
>> Cast away const for struct stoken (temporary).
>> * go-exp.y (parse_number): Make first argument const.
>> (parse_string_or_char): Likewise.
>> Make local variable 'tokstart' const.
>> (lex_one_token): Likewise for numerous locals called 'p'.
>> Cast away const for struct stoken (temporary).
>> * jv-exp.y (parse_number): Make first argument const.
>> Make local variables 'tokstart' and 'tokptr' const.
>> Cast away const for call to skip_quoted (temporary).
>> (yylex): Make local variable 'p' const.
>> Cast away const for struct stoken (temporary).
>> * m2-exp.y (parse_number): Make local variable 'p' const.
>> (yylex): Likewise for 'tokstart'.
>> Cast away const for struct stoken (temporary).
>> Make local variable 'p' const.
>> * macroexp.c (get_character_constant): Pass a const string
>> to c_parse_escape.
>> (get_string_literal): Likewise.
>> (macro_expand_next): Make first argument const.
>> Cast away const for init_shared_buffer.
>> * macroexp.h (macro_expand_next): Make first argument const.
>> * p-exp.y (yylex): Make a local copy of 'lexptr'.
>> Pass a const string to c_parse_escape.
>> Make local variables 'p' and 'namestart' const.
>> * parse.c (lexptr): Make const.
>> (prev_lexptr): Likewise.
>> (find_template_name_end): Return const.
>> Make argument const, too.
>> (parse_exp_in_context): Make first argument const.
>> Remove the entire const_hack.
>> (parse_exp_in_context_1): Make first argument const.
>> * parser-defs.h (find_template_name_end): Return const.
>> Make argument const, too.
>> (lexptr): Make const.
>> (prev_lexptr): Likewise.
>> * utils.c (parse_escape): Make second argument const.
>> * utils.h (parse_escape): Likewise.
>> diff --git a/gdb/c-exp.y b/gdb/c-exp.y
>> index 3a51878..ea19178 100644
>> --- a/gdb/c-exp.y
>> +++ b/gdb/c-exp.y
[snip]
>> @@ -1699,7 +1699,7 @@ check_parameter_typelist (VEC (type_ptr) *params)
>> /*** Needs some error checking for the float case ***/
>>
>> static int
>> -parse_number (char *p, int len, int parsed_float, YYSTYPE *putithere)
>> +parse_number (const char *buf, int len, int parsed_float, YYSTYPE *putithere)
>> {
>> /* FIXME: Shouldn't these be unsigned? We don't deal with negative values
>> here, and we do kind of silly things like cast to unsigned. */
>> @@ -1722,6 +1722,9 @@ parse_number (char *p, int len, int parsed_float, YYSTYPE *putithere)
>> struct type *signed_type;
>> struct type *unsigned_type;
>>
>> + char *p = alloca (len);
>> + memcpy (p, buf, len);
>
> I think the declaration of "p" should go together with the other
> declarations. And there should be a newline between the declaration and
> memcpy.
Yes, you are correct! I've fixed this. [See, I *did* warn about the
tediousness of these sort of operations. Fortunately, only a handful of
things escaped my attention.]
>> @@ -2723,7 +2727,7 @@ lex_one_token (void)
>> && (tokstart[namelen] == ' ' || tokstart[namelen] == '\t')
>> && ! scanning_macro_expansion ())
>> {
>> - char *p = tokstart + namelen + 1;
>> + const char *p = tokstart + namelen + 1;
>> while (*p == ' ' || *p == '\t')
>
> Newline between declaration and the "while" (you add this in the other
> hunks :-)).
Fixed. [Yes, I did fix the other occurrences since I was already editing
that line anyway.]
>> diff --git a/gdb/macroexp.c b/gdb/macroexp.c
>> index d88dac3..14c4994 100644
>> --- a/gdb/macroexp.c
>> +++ b/gdb/macroexp.c
>> @@ -360,8 +360,11 @@ get_character_constant (struct macro_buffer *tok, char *p, char *end)
>> }
>> else if (*p == '\\')
>> {
>> - p++;
>> - char_count += c_parse_escape (&p, NULL);
>> + const char *s, *o;
>> +
>> + s = o = ++p;
>
> This line is indented wrongly (only spaces).
Fixed.
>
>> + char_count += c_parse_escape (&s, NULL);
>> + p += s - o;
>> }
>> else
>> {
>> @@ -414,8 +417,11 @@ get_string_literal (struct macro_buffer *tok, char *p, char *end)
>> "constants."));
>> else if (*p == '\\')
>> {
>> - p++;
>> - c_parse_escape (&p, NULL);
>> + const char *s, *o;
>> +
>> + s = o = ++p;
>> + c_parse_escape (&s, NULL);
>
> Wrong indentation (only spaces).
>
Fixed.
>> + p += s - o;
>> }
>> else
>> p++;
>> diff --git a/gdb/parse.c b/gdb/parse.c
>> index 674342b..07c1765 100644
>> --- a/gdb/parse.c
>> +++ b/gdb/parse.c
>> @@ -1142,16 +1142,8 @@ parse_exp_in_context (const char **stringptr, CORE_ADDR pc,
>> const struct block *block,
>> int comma, int void_context_p, int *out_subexp)
>> {
>> - struct expression *expr;
>> - char *const_hack = *stringptr ? xstrdup (*stringptr) : NULL;
>> - char *orig = const_hack;
>> - struct cleanup *back_to = make_cleanup (xfree, const_hack);
>> -
>> - expr = parse_exp_in_context_1 (&const_hack, pc, block, comma,
>> + return parse_exp_in_context_1 (stringptr, pc, block, comma,
>> void_context_p, out_subexp);
>> - (*stringptr) += const_hack - orig;
>> - do_cleanups (back_to);
>> - return expr;
>
> Nice! :-)
>
Mission accomplished! :-)
>> }
>>
>> /* As for parse_exp_1, except that if VOID_CONTEXT_P, then
> Otherwise, looks good to me (thanks for doing that!), but I can't
> approve it :-).
For the record, here is update with the aforementioned fixes.
Thank you for taking a look!
Keith
[ChangeLog is above]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: constify-lexptr-2.patch
Type: text/x-patch
Size: 18993 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20131001/bf4fe293/attachment.bin>
More information about the Gdb-patches
mailing list