Re: [PATCH] libiberty: Initialize d_printing in all cplus_demangle_* functions.

[Mark Wielaard <mark@klomp.org>]

On Tue, 2017-03-14 at 01:26 +0000, Pedro Alves wrote:
> And there's a cplus_demangle_fill_component function to fill
> these in.  It's already used by cp-name-parser.y in other
> cases, via the fill_comp wrapper function.
> 
> Using that seems to work, testing the patch below on x86_64 Fedora 23
> shows no regressions.
> 
> From e55453c67bbe772ce001ea15b152f8dc44b8945e Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 13 Mar 2017 22:54:09 +0000
> Subject: [PATCH] gdb/cp-name-parser.y: Eliminate make_empty, use
>  cplus_demangle_fill_component
> 
> The demangler exports the cplus_demangle_fill_component function that
> clients should use to initialize demangle_component components that
> use the "s_binary" union member.  cp-name-parser.y uses it in some
> places, via the fill_comp wrapper, but not all.  Several places
> instead use a GDB-specific "make_empty" function.  Because this
> function does not call any of the demangler "fill" functions, we had
> to patch it recently to clear the allocated demangle_component's
> "d_printing" field, which is supposedly a "private" demangler field.
> To avoid such problems in the future, this commit switches those
> places to use "fill_comp" instead, and eliminates the "make_empty"
> function.

That looks good. But note that there is one behavioral change.
cplus_demangle_fill_component is defined as:

/* Fill in most component types with a left subtree and a right
   subtree.  Returns non-zero on success, zero on failure, such as an
   unrecognized or inappropriate component type.  */

And gdb/cp-name-parser.y fill_comp does:

  i = cplus_demangle_fill_component (ret, d_type, lhs, rhs);
  gdb_assert (i);

So you have an extra sanity check. Where before you might have silently
created a bogus demangle_component. Which I assume is what you want. But
it might depend on what gdb_assert precisely does and if the parser
input is normally "sane" or not.

Cheers,

Mark