Re: [PATCH 02/18] poison "new"

Jan Kratochvil
2013-10-10T19:15:08Z

On Wed, 09 Oct 2013 19:17:17 +0200, Ondrej Oprala wrote:
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -22,6 +22,43 @@
>
> 2013-10-09 Tom Tromey <tromey@redhat.com>
>
> + * bcache.c (new_bstr) : Renamed from new.
> + * buildsym.c(new_ctx) : Likewise.
> + * cleanups.c(new_cleanup) : Likewise.

Besides Pedro's comment on formatting of the ChangeLog entry do not post
ChangeLog entry already modified in the file. Post it only as a preceding
text (commit message). You can see here the entry is wrongly placed, new
entry should be always at the top of the file. Besides that it always causes
patch or merge conflicts as the top of ChangeLog files is continually
changing.

[...]
> --- a/gdb/dbxread.c
> +++ b/gdb/dbxread.c
[...]
> @@ -2799,16 +2799,17 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, char *name,
> }
>
> within_function = 0;
> - new = pop_context ();
> + new_ctx = pop_context ();
>
> /* Make a block for the local symbols within. */
> - block = finish_block (new->name, &local_symbols, new->old_blocks,
> - new->start_addr, new->start_addr + valu,
> + block = finish_block (new_ctx->name, &local_symbols,
> + new_ctx->old_blocks,
> + new_ctx->start_addr, new_ctx->start_addr + valu,
> objfile);
>
> /* For C++, set the block's scope. */
> - if (SYMBOL_LANGUAGE (new->name) == language_cplus)
> - cp_set_block_scope (new->name, block, &objfile->objfile_obstack);
> + if (SYMBOL_LANGUAGE (new_ctx->name) == language_cplus)
> + cp_set_block_scope (new_ctx->name, block, &objfile->objfile_obstack);

Line exceeds 80 columns.

>
> /* May be switching to an assembler file which may not be using
> block relative stabs, so reset the offset. */
[...]
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
[...]
> @@ -11030,12 +11030,12 @@ read_lexical_block_scope (struct die_info *die, struct dwarf2_cu *cu)
> child_die = sibling_die (child_die);
> }
> }
> - new = pop_context ();
> + new_ctx = pop_context ();
>
> if (local_symbols != NULL || using_directives != NULL)
> {
> struct block *block
> - = finish_block (0, &local_symbols, new->old_blocks, new->start_addr,
> + = finish_block (0, &local_symbols, new_ctx->old_blocks, new_ctx->start_addr,

Line exceeds 80 columns.

> highpc, objfile);
>
> /* Note that recording ranges after traversing children, as we
[...]
> --- a/gdb/stabsread.c
> +++ b/gdb/stabsread.c
[...]
> @@ -3184,7 +3184,7 @@ read_baseclasses (struct field_info *fip, char **pp, struct type *type,
> corresponding to this baseclass. Always zero in the absence of
> multiple inheritance. */
>
> - SET_FIELD_BITPOS (new->field, read_huge_number (pp, ',', &nbits, 0));
> + SET_FIELD_BITPOS (new_fields->field, read_huge_number (pp, ',', &nbits, 0));

Line exceeds 80 columns.

> if (nbits != 0)
> return 0;
> }

Otherwise the patch is fine with me. Please repost it to recheck the proper
GNU Coding Standards formatting of the long lines and the ChangeLog entry.

Ondrej is covered by the Red Hat copyright assignment. I guess you do not
have read/write access to the repository so I can do the check-in of some your
first patches.

Thanks,
Jan