Patchwork [v2] gdb: CTF support

login
register
mail settings
Submitter Weimin Pan
Date Oct. 1, 2019, 12:36 a.m.
Message ID <895f47d4-3e01-4d5a-474b-43dd2dd037b4@oracle.com>
Download mbox | patch
Permalink /patch/34753/
State New
Headers show

Comments

Weimin Pan - Oct. 1, 2019, 12:36 a.m.
On 9/29/2019 8:06 PM, Simon Marchi wrote:
 > Hi Weimin,
 >
 > I am done reading ctfread.c, I noted a few comments below.
 >
 > There are a few issues with styling as well, most importantly:
 >
 > - The indentation
 > - Use "= NULL" / "!= NULL" when testing a pointer
 >
 > Instead of pointing them out individually, I've fixed what I saw as I 
read
 > the patch, see the attached patch.  If you use it, please read through it
 > carefully and don't assume I haven't made mistakes :).

Hi Simon,

Thank you very much for the thorough review. I have read through the
modified patch which looks good and will be adopted. In addition, I
have made more changes to address the issues you raised. Please see
below and the updated patch (attached).

 >> +struct field_info
 >> +  {
 >> +    /* List of data member fields.  */
 >> +    std::vector<struct nextfield> fields;
 >> +
 >> +    /* Number of fields.  */
 >> +    int nfields = 0;
 >
 > This field seems unnecessary, as it represents the size of the vector 
above (which you can get with fields.size()).

You're right, the field is not needed and is removed.

 >
 >> +/* Callback to add member NAME to a struct/union type. TID is the type
 >> +   of struct/union member, OFFSET is the offset of member in bits
 >> +   (gdbarch_bits_big_endian(), and ARG contains the field_info.  */
 >
 > This comment (the gdbarch_bits_big_endian part) seems incomplete).

Updated.

 >
 >> +/* Callback to add member NAME of EVAL to an enumeration type.
 >> +   ARG contains the field_info.  */
 >> +
 >> +static int
 >> +ctf_add_enum_member_cb (const char *name, int eval, void *arg)
 >
 > I had to search why that variable was called EVAL.  It means "enum 
value", but I
 > automatically associate "eval" to "evaluate".  I'd suggest renaming 
it to "enum_value"
 > or "value" for clarity.

OK, using "enum_value".

 >
 >> +/* Add a new symbol entry, with its name from TP, its access index and
 >> +   domain from TP's kind, and its type from TPYE.  */
 >
 > This comment refers to TP, which doesn't exist, so it seems 
outdated.  Also,
 > watch the typo "TPYE".

Fixed.

 >
 >> +
 >> +static struct symbol *
 >> +new_symbol (ctf_context_t *ccp, struct type *type, ctf_id_t tid)
 >> +{
 >> +  struct objfile *objfile = ccp->of;
 >> +  ctf_file_t *fp = ccp->fp;
 >> +  struct symbol *sym = NULL;
 >> +
 >> +  const char *name = ctf_type_aname_raw (fp, tid);
 >> +  if (name)
 >> +    {
 >> +      sym = allocate_symbol (objfile);
 >> +      OBJSTAT (objfile, n_syms++);
 >> +
 >> +      SYMBOL_SET_LANGUAGE (sym, language_c, &objfile->objfile_obstack);
 >> +      SYMBOL_SET_NAMES (sym, xstrdup (name), strlen (name), 0, 
objfile);
 >
 > Looking at the code of ctf_type_aname_raw, it seems to return an 
allocated copy,
 > which needs to be freed eventually.

Yes, I've made the change in several places where the allocated copy is 
freed after
it's being dup'ed via obstack_strdup.

 > Also, instead of calling xstrdup, you can just pass true to the 
COPY_NAME parameter.

Good point, done.

 >> +/* Given a TID of kind CTF_K_INTEGER or CTF_K_FLOAT, find a 
representation
 >> +   and create the symbol for it.  */
 >> +
 >> +static struct type *
 >> +read_base_type (ctf_context_t *ccp, ctf_id_t tid)
 >> +{
 >> +  struct objfile *of = ccp->of;
 >> +  ctf_file_t *fp = ccp->fp;
 >> +  ctf_encoding_t cet;
 >> +  struct type *type = NULL;
 >> +  const char *name;
 >> +  uint32_t kind;
 >> +
 >> +  if (ctf_type_encoding (fp, tid, &cet))
 >> +    {
 >> +      complaint (_("ctf_type_encoding read_base_type failed - %s"),
 >> +                 ctf_errmsg (ctf_errno (fp)));
 >> +      return NULL;
 >> +    }
 >> +
 >> +  name = ctf_type_aname_raw (fp, tid);
 >> +  if (!name || (name && !strlen (name)))
 >
 > I think that can be written more simply (and in GNU style) as
 >
 >   if (name == NULL || strlen (name) == 0)

OK, updated in several places.

 >
 >> +/* Start a structure or union scope (definition) with TID and TP to 
create
 >> +   a type for the structure or union.
 >
 > TP seems to refer to something that doesn't exist (anymore?).  This 
occurs a few times
 > in the file.

All the references to TP have been cleaned up.

 >
 >> +
 >> +   Fill in the type's name and general properties. The members will 
not be
 >> +   processed, nor a symbol table entry be done until 
process_structure_type
 >> +   (assuming the type has a name).  */
 >> +
 >> +static struct type *
 >> +read_structure_type (ctf_context_t *ccp, ctf_id_t tid)
 >> +{
 >> +  struct objfile *of = ccp->of;
 >> +  ctf_file_t *fp = ccp->fp;
 >> +  struct type *type;
 >> +  const char *name;
 >> +  uint32_t kind;
 >
 > I'd suggest adding a gdb_assert to make sure that tid represents a 
struct or union
 > (and actually I'd suggest doing the same for all read_foo functions, 
make sure the
 > kind of the TID is what we expect).

Actually callers of these read_foo functions will call the appropriate 
function, based
on the tid's kind. For example, either ctf_add_type_cb or 
process_structure_type
calls read_structure_type only if tid's kind is CTF_K_STRUCT or 
CTF_K_STRUCT.

 >
 >> +  type = alloc_type (of);
 >> +  name = ctf_type_aname_raw (fp, tid);
 >
 > ctf_type_aname_raw returns an allocated string.  Is it ever free'd?
 >
 > This applies to other uses of ctf_type_aname_raw.

Done, please see above.

 >
 >> +....
 >> +/* Given a tid of CTF_K_STRUCT or CTF_K_UNION, process all its members
 >> +   and create the symbol for it.  */
 >> +
 >> +static void
 >> +process_struct_members (ctf_context_t *ccp,
 >> +                        ctf_id_t tid,
 >> +                        struct type *type)
 >> +{
 >> +  ctf_file_t *fp = ccp->fp;
 >> +  struct field_info fi;> +
 >> +  fi.cur_context.fp = fp;
 >> +  fi.cur_context.of = ccp->of;
 >> +  fi.cur_context.builder = ccp->builder;
 >
 > I might be missing something, but I find it odd to copy the whole 
ctf_context_t structure,
 > couldn't you just make field_info::cur_context a pointer, and just 
assign CCP to it?

Yes, it is better and is done.

 >
 >> +/* Read TID of kind CTF_K_VOLATILE with base type BTID. */
 >> +
 >> +static struct type *
 >> +read_volatile_type (ctf_context_t *ccp, ctf_id_t tid, ctf_id_t btid)
 >> +{
 >> +  struct objfile *objfile = ccp->of;
 >> +  ctf_file_t *fp = ccp->fp;
 >> +  struct type *base_type, *cv_type;
 >> +
 >> +  base_type = get_tid_type (objfile, btid);
 >> +  if (!base_type)
 >> +    {
 >
 > In read_const_type and read_restrict_type, you call read_type_record 
to read
 > in the type if it hasn't been read yet.  Is there a reason you don't 
do it here?

No, I need to do the same, i.e. calling read_type_record, in 
read_volatile_type.

 >
 >> +/* Get text segment base for OBJFILE, TSIZE contains the segment 
size.  */
 >> +
 >> +static CORE_ADDR
 >> +get_of_text_range (struct objfile *of, int *tsize)
 >
 > I'd suggest renaming this to get_objfile_text_range.  A few 
characters more,
 > but a lot clearer IMO.

OK.

 >
 >> +/* Read in full symbols for PST, and anything it depends on.  */
 >> +
 >> +static void
 >> +psymtab_to_symtab (struct partial_symtab *pst)
 >> +{
 >> +  struct symbol *sym;
 >> +  ctf_context_t *ccp;
 >> +
 >> +  if (pst->readin == 1)
 >> +    return;
 >
 > This should never happen (it's checked in ctf_read_symtab), so I 
would use:
 >
 >   gdb_assert (!pst->readin);

It makes sense, done.

 >
 >> +static struct partial_symtab *
 >> +create_partial_symtab (const char *name,
 >> +                       ctf_file_t *cfp,
 >> +                       struct objfile *objfile)
 >> +{
 >> +  struct partial_symtab *pst;
 >> +  static ctf_context_t ccx;
 >> +
 >> +  pst = start_psymtab_common (objfile, name, 0);
 >> +
 >> +  ccx.fp = cfp;
 >> +  ccx.of = objfile;
 >> +  pst->read_symtab_private = (void *)&ccx;
 >
 > Hmm that looks fishy.  It looks like this is taking the address of a
 > local variable for something that will be used after the current function
 > will have returned.

Sorry, but it's a "static" local variable.

 >
 >> +...
 >> +  /* Scan CTF object and function sections which correspond to each
 >> +     STT_FUNC or STT_OBJECT entry in the symbol table,
 >> +     pick up what init_symtab has done.  */
 >> +  for (unsigned long idx = 0; ; idx++)
 >> +    {
 >> +      ctf_id_t tid;
 >> +      if ((tid = ctf_lookup_by_symbol (cfp, idx)) == CTF_ERR)
 >> +        {
 >> +    if (ctf_errno (cfp) == EINVAL
 >> +           || ctf_errno (cfp) == ECTF_NOSYMTAB)
 >> +        // case ECTF_SYMRANGE:
 >> +      break;
 >
 > Is this comment (the "case ECTF_SYMRANGE:" comment) a leftover?

Yes, it is and is taken out.

 >
 > Also, if these cases happen, it means the debug information is not 
valid/corrupted?
 > Should there be an error printed to the user, should we maybe call 
error()?

We use either EINVAL or ECTF_NOSYMTAB to signal end of the section has
been reached. I have added such a comment in the code.

Weimin
Simon Marchi - Oct. 2, 2019, 2:47 a.m.
On 2019-09-30 8:36 p.m., Weimin Pan wrote:
> Thank you very much for the thorough review. I have read through the
> modified patch which looks good and will be adopted. In addition, I
> have made more changes to address the issues you raised. Please see
> below and the updated patch (attached).

Great thanks!

I noticed at least one case of leading spaces that should be tab in the attached
patch, so maybe do:

  grep '^        ' ctfread.c

to catch them.

> Yes, I've made the change in several places where the allocated copy is 
> freed after
> it's being dup'ed via obstack_strdup.

Ok thanks.  Actually, can you please use gdb::unique_xmalloc_pointer to
manage that memory?  We're trying to minimize the manual memory management
in GDB.  You'd do:

  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));

and use `name.get ()` instead of name.  It will get free'd automatically
when exiting scope.

> Actually callers of these read_foo functions will call the appropriate 
> function, based
> on the tid's kind. For example, either ctf_add_type_cb or 
> process_structure_type
> calls read_structure_type only if tid's kind is CTF_K_STRUCT or 
> CTF_K_STRUCT.

Ok, I was hoping it would make the code more robust in case someone does
some refactor in the future and one of these functions get called with an
object of the wrong type, but maybe it would be overkill.

>  > Hmm that looks fishy.  It looks like this is taking the address of a
>  > local variable for something that will be used after the current function
>  > will have returned.
> 
> Sorry, but it's a "static" local variable.

Oh ok, I did miss that.

I think we should avoid this.  What happens, for example, in this case:

1. Read psymtab for objfile 1
2. Read psymtab for objfile 2
3. Expand psymtab to symtab for objfile 1

From what I understand, step #2 will overwrite the static variable, such that
it won't be valid anymore in step #3.  I think it would be better to allocate the
context structure dynamically and store it as the objfile, like dwarf2read does.

Thanks,

Simon
Weimin Pan - Oct. 2, 2019, 11:09 p.m.
On 10/1/2019 7:47 PM, Simon Marchi wrote:
 > On 2019-09-30 8:36 p.m., Weimin Pan wrote:
 >> Thank you very much for the thorough review. I have read through the
 >> modified patch which looks good and will be adopted. In addition, I
 >> have made more changes to address the issues you raised. Please see
 >> below and the updated patch (attached).
 >
 > Great thanks!
 >
 > I noticed at least one case of leading spaces that should be tab in 
the attached
 > patch, so maybe do:
 >
 >   grep '^        ' ctfread.c
 >
 > to catch them.

Hi Simon,

Thanks, I think it's now clean after fixing the one below:

1247c1247
<           sym = add_stt_func (ccp, i);
---
 >         sym = add_stt_func (ccp, i);

 >
 >> Yes, I've made the change in several places where the allocated copy is
 >> freed after
 >> it's being dup'ed via obstack_strdup.
 >
 > Ok thanks.  Actually, can you please use gdb::unique_xmalloc_pointer to
 > manage that memory?  We're trying to minimize the manual memory 
management
 > in GDB.  You'd do:
 >
 >   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
 >
 > and use `name.get ()` instead of name.  It will get free'd automatically
 > when exiting scope.

It doesn't seem using gdb::unique_xmalloc_pointer is appropriate in 
these cases
because we want to keep these symbol/type names around in the symbol table.

 >
 >>  > Hmm that looks fishy.  It looks like this is taking the address of a
 >>  > local variable for something that will be used after the current 
function
 >>  > will have returned.
 >>
 >> Sorry, but it's a "static" local variable.
 >
 > Oh ok, I did miss that.
 >
 > I think we should avoid this.  What happens, for example, in this case:
 >
 > 1. Read psymtab for objfile 1
 > 2. Read psymtab for objfile 2
 > 3. Expand psymtab to symtab for objfile 1
 >
 > From what I understand, step #2 will overwrite the static variable, 
such that
 > it won't be valid anymore in step #3.  I think it would be better to 
allocate the
 > context structure dynamically and store it as the objfile, like 
dwarf2read does.

Yes, you're right. I've made the following changes:

1322c1322
<   static ctf_context_t ccx;
---
 >   ctf_context_t *ccx;
1326,1328c1326,1329
<   ccx.fp = cfp;
<   ccx.of = objfile;
<   pst->read_symtab_private = (void *) &ccx;
---
 >   ccx = XOBNEW (&objfile->objfile_obstack, ctf_context_t);
 >   ccx->fp = cfp;
 >   ccx->of = objfile;
 >   pst->read_symtab_private = (void *) ccx;

Many thanks,
Weimin
Simon Marchi - Oct. 3, 2019, 12:40 p.m.
On 2019-10-02 7:09 p.m., Wei-min Pan wrote:
> It doesn't seem using gdb::unique_xmalloc_pointer is appropriate in 
> these cases
> because we want to keep these symbol/type names around in the symbol table.

I mean, instead of calling `free` by hand, that copy could be managed with
a gdb::unique_xmalloc_pointer.  The name duplicated into the symbol table
won't change.

Simon
Weimin Pan - Oct. 3, 2019, 5:58 p.m.
On 10/3/2019 5:40 AM, Simon Marchi wrote:
> On 2019-10-02 7:09 p.m., Wei-min Pan wrote:
>> It doesn't seem using gdb::unique_xmalloc_pointer is appropriate in
>> these cases
>> because we want to keep these symbol/type names around in the symbol table.
> I mean, instead of calling `free` by hand, that copy could be managed with
> a gdb::unique_xmalloc_pointer.  The name duplicated into the symbol table
> won't change.

But the `free` call is needed to free up space allocated by libctf's 
ctf_type_aname_raw.

> Simon
>
Simon Marchi - Oct. 3, 2019, 6:01 p.m.
On 2019-10-03 1:58 p.m., Wei-min Pan wrote:
> But the `free` call is needed to free up space allocated by libctf's ctf_type_aname_raw.

That's the point of gdb::unique_xmalloc_pointer: it will automatically call xfree (free) when
on scope exit, free'ing this copy.  It's preferred to use this instead of manually calling
xfree, because it makes it harder to forget to free the memory (or to free it twice).

Simon
Weimin Pan - Oct. 3, 2019, 6:21 p.m.
On 10/3/2019 11:01 AM, Simon Marchi wrote:
 > On 2019-10-03 1:58 p.m., Wei-min Pan wrote:
 >> But the `free` call is needed to free up space allocated by libctf's 
ctf_type_aname_raw.
 >
 > That's the point of gdb::unique_xmalloc_pointer: it will 
automatically call xfree (free) when
 > on scope exit, free'ing this copy.  It's preferred to use this 
instead of manually calling
 > xfree, because it makes it harder to forget to free the memory (or to 
free it twice).

Let's use an example (checking omitted):

We're replacing:
   name = ctf_type_aname_raw (fp, tid);
   TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name);
   free (name);

with
   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
   TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name.get ());

The allocated copy from ctf_type_aname_raw is not freed. Or did I miss 
something?

Weimin
Simon Marchi - Oct. 3, 2019, 6:31 p.m.
On 2019-10-03 2:21 p.m., Wei-min Pan wrote:
> 
> Let's use an example (checking omitted):
> 
> We're replacing:
>   name = ctf_type_aname_raw (fp, tid);
>   TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name);
>   free (name);
> 
> with
>   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
>   TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name.get ());
> 
> The allocated copy from ctf_type_aname_raw is not freed. Or did I miss something?
> 
> Weimin

Yes, the second snippet frees the copy returned by ctf_type_aname_raw.

It might just be that you are not familiar with the concept of std::unique_ptr in C++:

  https://en.cppreference.com/w/cpp/memory/unique_ptr

It's a wrapper around a simple pointer that automatically calls a deleter function
when it goes out of scope.  gdb::unique_xmalloc_ptr is a specialization of std::unique_ptr
that uses xfree as the deleter function.

Here's a similar use:

  https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/fbsd-tdep.c;h=9422e3c1a7e1a65c76b088f42bb5986bd13a089f;hb=HEAD#l1530

`fbsd_core_vnode_path` return a pointer to an allocated C String, which `cwd` wraps.  When
it goes out of scope, `cwd` automatically calls `xfree` with the pointer.

Simon
Weimin Pan - Oct. 3, 2019, 6:53 p.m.
On 10/3/2019 11:31 AM, Simon Marchi wrote:
> On 2019-10-03 2:21 p.m., Wei-min Pan wrote:
>> Let's use an example (checking omitted):
>>
>> We're replacing:
>>    name = ctf_type_aname_raw (fp, tid);
>>    TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name);
>>    free (name);
>>
>> with
>>    gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
>>    TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name.get ());
>>
>> The allocated copy from ctf_type_aname_raw is not freed. Or did I miss something?
>>
>> Weimin
> Yes, the second snippet frees the copy returned by ctf_type_aname_raw.
>
> It might just be that you are not familiar with the concept of std::unique_ptr in C++:
>
>    https://en.cppreference.com/w/cpp/memory/unique_ptr
>
> It's a wrapper around a simple pointer that automatically calls a deleter function
> when it goes out of scope.  gdb::unique_xmalloc_ptr is a specialization of std::unique_ptr
> that uses xfree as the deleter function.
>
> Here's a similar use:
>
>    https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/fbsd-tdep.c;h=9422e3c1a7e1a65c76b088f42bb5986bd13a089f;hb=HEAD#l1530
>
> `fbsd_core_vnode_path` return a pointer to an allocated C String, which `cwd` wraps.  When
> it goes out of scope, `cwd` automatically calls `xfree` with the pointer.

Oh, thanks for the explanation, reference and example. Will switch to 
using gdb::unique_xmalloc_ptr<> instead.

Weimin
>
> Simon
Weimin Pan - Oct. 3, 2019, 8:33 p.m.
On 10/3/2019 11:53 AM, Wei-min Pan wrote:
>
> On 10/3/2019 11:31 AM, Simon Marchi wrote:
>> On 2019-10-03 2:21 p.m., Wei-min Pan wrote:
>>> Let's use an example (checking omitted):
>>>
>>> We're replacing:
>>>    name = ctf_type_aname_raw (fp, tid);
>>>    TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name);
>>>    free (name);
>>>
>>> with
>>>    gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
>>>    TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name.get ()); 
>>>
>>>
>>> The allocated copy from ctf_type_aname_raw is not freed. Or did I 
>>> miss something?
>>>
>>> Weimin
>> Yes, the second snippet frees the copy returned by ctf_type_aname_raw.
>>
>> It might just be that you are not familiar with the concept of 
>> std::unique_ptr in C++:
>>
>>    https://en.cppreference.com/w/cpp/memory/unique_ptr
>>
>> It's a wrapper around a simple pointer that automatically calls a 
>> deleter function
>> when it goes out of scope.  gdb::unique_xmalloc_ptr is a 
>> specialization of std::unique_ptr
>> that uses xfree as the deleter function.
>>
>> Here's a similar use:
>>
>> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/fbsd-tdep.c;h=9422e3c1a7e1a65c76b088f42bb5986bd13a089f;hb=HEAD#l1530
>>
>> `fbsd_core_vnode_path` return a pointer to an allocated C String, 
>> which `cwd` wraps.  When
>> it goes out of scope, `cwd` automatically calls `xfree` with the 
>> pointer.
>
> Oh, thanks for the explanation, reference and example. Will switch to 
> using gdb::unique_xmalloc_ptr<> instead.

Hi Simon,

I've switched to using gdb::unique_xmalloc_ptr, please see the attached 
patch, and ran
the CTF test suite successfully. Will submit a revised patch upstream 
shortly.

Thanks again for your reviews and help.

Weimin
405c410
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
---
>   char *name = ctf_type_aname_raw (fp, tid);
412c417
<       SYMBOL_SET_NAMES (sym, name.get (), strlen (name.get ()), 1, objfile);
---
>       SYMBOL_SET_NAMES (sym, name, strlen (name), 1, objfile);
414a420
>       free (name);
478,479c484,485
<   gdb::unique_xmalloc_ptr<char> copied_name (ctf_type_aname_raw (fp, tid));
<   if (copied_name == NULL || strlen (copied_name.get ()) == 0)
---
>   name = ctf_type_aname_raw (fp, tid);
>   if (name == NULL || strlen (name) == 0)
487c493,497
<     name = obstack_strdup (&of->objfile_obstack, copied_name.get ());
---
>     {
>       char *dup_name = name;
>       name = obstack_strdup (&of->objfile_obstack, name);
>       free (dup_name);
>     }
561a572
>   char *name;
566,568c577,582
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
<   if (name != NULL && strlen (name.get() ) != 0)
<     TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name.get ());
---
>   name = ctf_type_aname_raw (fp, tid);
>   if (name != NULL && strlen (name) != 0)
>     {
>       TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name);
>       free (name);
>     }
617c631
<   struct objfile *of = ccp->of;
---
>   struct objfile *objfile = ccp->of;
620a635
>   char *name;
622c637
<   type = alloc_type (of);
---
>   type = alloc_type (objfile);
624,626c639,644
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
<   if (name != NULL && strlen (name.get ()) != 0)
<     TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name.get ());
---
>   name = ctf_type_aname_raw (fp, tid);
>   if (name != NULL && strlen (name) != 0)
>     {
>       TYPE_NAME (type) = obstack_strdup (&objfile->objfile_obstack, name);
>       free (name);
>     }
630c648
<   rettype = get_tid_type (of, cfi.ctc_return);
---
>   rettype = get_tid_type (objfile, cfi.ctc_return);
634c652
<   return set_tid_type (of, tid, type);
---
>   return set_tid_type (objfile, tid, type);
646a665
>   char *name;
650,652c669,674
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
<   if (name != NULL && strlen (name.get ()) != 0)
<     TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name.get ());
---
>   name = ctf_type_aname_raw (fp, tid);
>   if (name != NULL && strlen (name) != 0)
>     {
>       TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name);
>       free (name);
>     }
880a903
>   char *name;
903,907c926,929
< 	{ 
< 	  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
< 	  btid = ctf_type_reference (fp, tid);
< 	  type = read_typedef_type (ccp, tid, btid, name.get ());
< 	} 
---
> 	name = ctf_type_aname_raw (fp, tid);
> 	btid = ctf_type_reference (fp, tid);
> 	type = read_typedef_type (ccp, tid, btid, name);
> 	free (name);
1088a1111
>   char *name;
1103c1126
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (ccp->fp, idx));
---
>   name = ctf_type_aname_raw (ccp->fp, idx);
1105a1129
>   free (name);
1321a1346
>   char *name;
1326,1327c1351,1352
<   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (ccp->fp, tid));
<   if (name == NULL || strlen (name.get ()) == 0)
---
>   name = ctf_type_aname_raw (ccp->fp, tid);
>   if (name == NULL || strlen (name) == 0)
1368c1393
<     add_psymbol_to_list (name.get (), strlen (name.get ()), true,
---
>     add_psymbol_to_list (name, strlen (name), true,
1372a1398
>   free (name);
1425c1451
<       gdb::unique_xmalloc_ptr<char> tname (ctf_type_aname_raw (cfp, tid));
---
>       char *tname = ctf_type_aname_raw (cfp, tid);
1448c1474
<       add_psymbol_to_list (tname.get (), strlen (tname.get ()), true,
---
>       add_psymbol_to_list (tname, strlen (tname), true,
1451a1478
>       free (tname);

Patch

diff --git a/gdb/ctfread.c b/gdb/ctfread.c
index 9b6c70e..137d16b 100644
--- a/gdb/ctfread.c
+++ b/gdb/ctfread.c
@@ -106,29 +106,26 @@  struct nextfield
 };
 
 struct field_info
-  {
-    /* List of data member fields.  */
-    std::vector<struct nextfield> fields;
+{
+  /* List of data member fields.  */
+  std::vector<struct nextfield> fields;
 
-    /* Number of fields.  */
-    int nfields = 0;
+  /* Context.  */
+  ctf_context_t *cur_context;
 
-    /* Context.  */
-    ctf_context_t cur_context;
+  /* Parent type.  */
+  struct type *ptype;
 
-    /* Parent type.  */
-    struct type *ptype;
+  /* typedefs defined inside this class.  TYPEDEF_FIELD_LIST contains head
+     of a NULL terminated list of TYPEDEF_FIELD_LIST_COUNT elements.  */
+  std::vector<struct decl_field> typedef_field_list;
 
-    /* typedefs defined inside this class.  TYPEDEF_FIELD_LIST contains head
-       of a NULL terminated list of TYPEDEF_FIELD_LIST_COUNT elements.  */
-    std::vector<struct decl_field> typedef_field_list;
+  /* Nested types defined by this struct and the number of elements in
+     this list.  */
+  std::vector<struct decl_field> nested_types_list;
+};
 
-    /* Nested types defined by this struct and the number of elements in
-       this list.  */
-    std::vector<struct decl_field> nested_types_list;
-  };
 
-
 /* Local function prototypes */
 
 static void psymtab_to_symtab (struct partial_symtab *);
@@ -138,24 +135,24 @@  static int ctf_add_type_cb (ctf_id_t tid, void *arg);
 static struct type *read_array_type (ctf_context_t *ccp, ctf_id_t tid);
 
 static struct type *read_pointer_type (ctf_context_t *ccp, ctf_id_t tid,
-                                       ctf_id_t btid);
+				       ctf_id_t btid);
 
 static struct type *read_structure_type (ctf_context_t *ccp, ctf_id_t tid);
 
 static struct type *read_enum_type (ctf_context_t *ccp, ctf_id_t tid);
 
 static struct type *read_typedef_type (ctf_context_t *ccp, ctf_id_t tid,
-                                       ctf_id_t btid, const char *name);
+				       ctf_id_t btid, const char *name);
 
 static struct type *read_type_record (ctf_context_t *ccp, ctf_id_t tid);
 
 static void process_structure_type (ctf_context_t *ccp, ctf_id_t tid);
 
 static void process_struct_members (ctf_context_t *ccp, ctf_id_t tid,
-                        struct type *type);
+				    struct type *type);
 
-static struct symbol * new_symbol (ctf_context_t *ccp, struct type *type,
-                                   ctf_id_t tid);
+static struct symbol *new_symbol (ctf_context_t *ccp, struct type *type,
+				  ctf_id_t tid);
 
 struct ctf_tid_and_type
 {
@@ -198,8 +195,8 @@  set_tid_type (struct objfile *of, ctf_id_t tid, struct type *typ)
   if (htab == NULL)
     {
       htab = htab_create_alloc (1, tid_and_type_hash,
-                                tid_and_type_eq,
-                                NULL, xcalloc, xfree);
+				tid_and_type_eq,
+				NULL, xcalloc, xfree);
       ctf_tid_key.set (of, htab);
     }
 
@@ -209,7 +206,7 @@  set_tid_type (struct objfile *of, ctf_id_t tid, struct type *typ)
   slot = (struct ctf_tid_and_type **) htab_find_slot (htab, &ids, INSERT);
   if (*slot)
     complaint (_("An internal GDB problem: ctf_ id_t %ld type already set"),
-               (tid));
+	       (tid));
   *slot = XOBNEW (&of->objfile_obstack, struct ctf_tid_and_type);
   **slot = ids;
   return typ;
@@ -248,9 +245,7 @@  get_bitsize (ctf_file_t *fp, ctf_id_t tid, uint32_t kind)
       || kind == CTF_K_FLOAT)
       && ctf_type_reference (fp, tid) != CTF_ERR
       && ctf_type_encoding (fp, tid, &cet) != CTF_ERR)
-    {
-      return cet.cte_bits;
-    }
+    return cet.cte_bits;
 
   return 0;
 }
@@ -276,15 +271,15 @@  set_symbol_address (struct objfile *of, struct symbol *sym, const char *name)
 static void
 attach_fields_to_type (struct field_info *fip, struct type *type)
 {
-  int nfields = fip->nfields;
+  int nfields = fip->fields.size ();
 
-  if (!nfields)
+  if (nfields == 0)
     return;
 
   /* Record the field count, allocate space for the array of fields.  */
   TYPE_NFIELDS (type) = nfields;
-  TYPE_FIELDS (type) = (struct field *)
-    TYPE_ZALLOC (type, sizeof (struct field) * nfields);
+  TYPE_FIELDS (type)
+    = (struct field *) TYPE_ZALLOC (type, sizeof (struct field) * nfields);
 
   /* Copy the saved-up fields into the field vector.  */
   for (int i = 0; i < nfields; ++i)
@@ -300,16 +295,16 @@  attach_fields_to_type (struct field_info *fip, struct type *type)
 
 static struct type *
 ctf_init_float_type (struct objfile *objfile,
-                     int bits,
-                     const char *name,
-                     const char *name_hint)
+		     int bits,
+		     const char *name,
+		     const char *name_hint)
 {
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   const struct floatformat **format;
   struct type *type;
 
   format = gdbarch_floatformat_for_type (gdbarch, name_hint, bits);
-  if (format)
+  if (format != NULL)
     type = init_float_type (objfile, bits, name, format);
   else
     type = init_type (objfile, TYPE_CODE_ERROR, bits, name);
@@ -318,17 +313,17 @@  ctf_init_float_type (struct objfile *objfile,
 }
 
 /* Callback to add member NAME to a struct/union type. TID is the type
-   of struct/union member, OFFSET is the offset of member in bits
-   (gdbarch_bits_big_endian(), and ARG contains the field_info.  */
+   of struct/union member, OFFSET is the offset of member in bits,
+   and ARG contains the field_info.  */
 
 static int
 ctf_add_member_cb (const char *name,
-                   ctf_id_t tid,
-                   unsigned long offset,
-                   void *arg)
+		   ctf_id_t tid,
+		   unsigned long offset,
+		   void *arg)
 {
-  struct field_info *fip = (struct field_info *)arg;
-  ctf_context_t *ccp = &fip->cur_context;
+  struct field_info *fip = (struct field_info *) arg;
+  ctf_context_t *ccp = fip->cur_context;
   struct nextfield new_field;
   struct field *fp;
   struct type *t;
@@ -339,28 +334,25 @@  ctf_add_member_cb (const char *name,
 
   kind = ctf_type_kind (ccp->fp, tid);
   t = get_tid_type (ccp->of, tid);
-  if (!t)
+  if (t == NULL)
     {
       t = read_type_record (ccp, tid);
-      if (!t)
-        {
-          complaint (_("ctf_add_member_cb: %s has NO type (%ld)"), name, tid);
-          t = objfile_type (ccp->of)->builtin_error;
-          set_tid_type (ccp->of, tid, t);
-        }
+      if (t == NULL)
+	{
+	  complaint (_("ctf_add_member_cb: %s has NO type (%ld)"), name, tid);
+	  t = objfile_type (ccp->of)->builtin_error;
+	  set_tid_type (ccp->of, tid, t);
+	}
     }
 
   if (kind == CTF_K_STRUCT || kind == CTF_K_UNION)
-    {
-      process_struct_members (ccp, tid, t);
-    }
+    process_struct_members (ccp, tid, t);
 
   FIELD_TYPE (*fp) = t;
   SET_FIELD_BITPOS (*fp, offset / TARGET_CHAR_BIT);
   FIELD_BITSIZE (*fp) = get_bitsize (ccp->fp, tid, kind);
 
   fip->fields.emplace_back (new_field);
-  fip->nfields++;
 
   return 0;
 }
@@ -369,20 +361,20 @@  ctf_add_member_cb (const char *name,
    ARG contains the field_info.  */
 
 static int
-ctf_add_enum_member_cb (const char *name, int eval, void *arg)
+ctf_add_enum_member_cb (const char *name, int enum_value, void *arg)
 {
-  struct field_info *fip = (struct field_info *)arg;
+  struct field_info *fip = (struct field_info *) arg;
   struct nextfield new_field;
   struct field *fp;
-  ctf_context_t *ccp = &fip->cur_context;
+  ctf_context_t *ccp = fip->cur_context;
 
   fp = &new_field.field;
   FIELD_NAME (*fp) = name;
   FIELD_TYPE (*fp) = NULL;
-  SET_FIELD_ENUMVAL (*fp, eval);
+  SET_FIELD_ENUMVAL (*fp, enum_value);
   FIELD_BITSIZE (*fp) = 0;
 
-  if (name)
+  if (name != NULL)
     {
       struct symbol *sym = allocate_symbol (ccp->of);
       OBJSTAT (ccp->of, n_syms++);
@@ -396,13 +388,12 @@  ctf_add_enum_member_cb (const char *name, int eval, void *arg)
     }
 
   fip->fields.emplace_back (new_field);
-  fip->nfields++;
 
   return 0;
 }
 
-/* Add a new symbol entry, with its name from TP, its access index and
-   domain from TP's kind, and its type from TPYE.  */
+/* Add a new symbol entry, with its name from TID, its access index and
+   domain from TID's kind, and its type from TYPE.  */
 
 static struct symbol *
 new_symbol (ctf_context_t *ccp, struct type *type, ctf_id_t tid)
@@ -411,16 +402,17 @@  new_symbol (ctf_context_t *ccp, struct type *type, ctf_id_t tid)
   ctf_file_t *fp = ccp->fp;
   struct symbol *sym = NULL;
 
-  const char *name = ctf_type_aname_raw (fp, tid);
-  if (name)
+  char *name = ctf_type_aname_raw (fp, tid);
+  if (name != NULL)
     {
       sym = allocate_symbol (objfile);
       OBJSTAT (objfile, n_syms++);
 
       SYMBOL_SET_LANGUAGE (sym, language_c, &objfile->objfile_obstack);
-      SYMBOL_SET_NAMES (sym, xstrdup (name), strlen (name), 0, objfile);
+      SYMBOL_SET_NAMES (sym, name, strlen (name), 1, objfile);
       SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
       SYMBOL_ACLASS_INDEX (sym) = LOC_OPTIMIZED_OUT;
+      free (name);
 
       if (type != NULL)
 	SYMBOL_TYPE (sym) = type;
@@ -428,33 +420,33 @@  new_symbol (ctf_context_t *ccp, struct type *type, ctf_id_t tid)
       uint32_t kind = ctf_type_kind (fp, tid);
       switch (kind)
 	{
-      	  case CTF_K_STRUCT:
-      	  case CTF_K_UNION:
-      	  case CTF_K_ENUM:
+	  case CTF_K_STRUCT:
+	  case CTF_K_UNION:
+	  case CTF_K_ENUM:
 	    SYMBOL_ACLASS_INDEX (sym) = LOC_TYPEDEF;
 	    SYMBOL_DOMAIN (sym) = STRUCT_DOMAIN;
 	    break;
-      	  case CTF_K_FUNCTION:
+	  case CTF_K_FUNCTION:
 	    SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
 	    break;
-      	  case CTF_K_CONST:
+	  case CTF_K_CONST:
 	    if (TYPE_CODE (SYMBOL_TYPE (sym)) == TYPE_CODE_VOID)
 	      SYMBOL_TYPE (sym) = objfile_type (objfile)->builtin_int;
 	    break;
-      	  case CTF_K_TYPEDEF:
-      	  case CTF_K_INTEGER:
-      	  case CTF_K_FLOAT:
+	  case CTF_K_TYPEDEF:
+	  case CTF_K_INTEGER:
+	  case CTF_K_FLOAT:
 	    SYMBOL_ACLASS_INDEX (sym) = LOC_TYPEDEF;
 	    SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
 	    break;
-      	  case CTF_K_POINTER:
+	  case CTF_K_POINTER:
 	    break;
-      	  case CTF_K_VOLATILE:
-      	  case CTF_K_RESTRICT:
+	  case CTF_K_VOLATILE:
+	  case CTF_K_RESTRICT:
 	    break;
-      	  case CTF_K_SLICE:
-      	  case CTF_K_ARRAY:
-      	  case CTF_K_UNKNOWN:
+	  case CTF_K_SLICE:
+	  case CTF_K_ARRAY:
+	  case CTF_K_UNKNOWN:
 	    break;
 	}
 
@@ -474,23 +466,29 @@  read_base_type (ctf_context_t *ccp, ctf_id_t tid)
   ctf_file_t *fp = ccp->fp;
   ctf_encoding_t cet;
   struct type *type = NULL;
-  const char *name;
+  char *name;
   uint32_t kind;
 
   if (ctf_type_encoding (fp, tid, &cet))
     {
       complaint (_("ctf_type_encoding read_base_type failed - %s"),
-                 ctf_errmsg (ctf_errno (fp)));
+		 ctf_errmsg (ctf_errno (fp)));
       return NULL;
     }
 
   name = ctf_type_aname_raw (fp, tid);
-  if (!name || (name && !strlen (name)))
+  if (name == NULL || strlen (name) == 0)
     {
       name = ctf_type_aname (fp, tid);
-      if (!name)
-        complaint (_("ctf_type_aname read_base_type failed - %s"),
-                   ctf_errmsg (ctf_errno (fp)));
+      if (name == NULL)
+	complaint (_("ctf_type_aname read_base_type failed - %s"),
+		   ctf_errmsg (ctf_errno (fp)));
+    }
+  else
+    {
+      char *dup_name = name;
+      name = obstack_strdup (&of->objfile_obstack, name);
+      free (dup_name);
     }
 
   kind = ctf_type_kind (fp, tid);
@@ -503,10 +501,10 @@  read_base_type (ctf_context_t *ccp, ctf_id_t tid)
       ischar = cet.cte_format & CTF_INT_CHAR;
       isbool = cet.cte_format & CTF_INT_BOOL;
       if (ischar)
-      	type = init_character_type (of, TARGET_CHAR_BIT, !issigned, name);
+	type = init_character_type (of, TARGET_CHAR_BIT, !issigned, name);
       else if (isbool)
-      	type = init_boolean_type (of, gdbarch_int_bit (gdbarch),
-                                  !issigned, name);
+	type = init_boolean_type (of, gdbarch_int_bit (gdbarch),
+				  !issigned, name);
       else
 	{
 	  int bits;
@@ -514,25 +512,23 @@  read_base_type (ctf_context_t *ccp, ctf_id_t tid)
 	    bits = cet.cte_bits;
 	  else
 	    bits = gdbarch_int_bit (gdbarch);
-      	  type = init_integer_type (of, bits, !issigned, name);
+	  type = init_integer_type (of, bits, !issigned, name);
 	}
     }
   else if (kind == CTF_K_FLOAT)
     {
       uint32_t isflt;
       isflt = !((cet.cte_format & CTF_FP_IMAGRY) == CTF_FP_IMAGRY
-                 || (cet.cte_format & CTF_FP_DIMAGRY) == CTF_FP_DIMAGRY
-                 || (cet.cte_format & CTF_FP_LDIMAGRY) == CTF_FP_LDIMAGRY);
+		 || (cet.cte_format & CTF_FP_DIMAGRY) == CTF_FP_DIMAGRY
+		 || (cet.cte_format & CTF_FP_LDIMAGRY) == CTF_FP_LDIMAGRY);
       if (isflt)
-        {
-	  type = ctf_init_float_type (of, cet.cte_bits, name, name);
-        }
+	type = ctf_init_float_type (of, cet.cte_bits, name, name);
       else
-        {
-	  struct type *t;
-	  t = ctf_init_float_type (of, cet.cte_bits / 2, NULL, name);
-      	  type = init_complex_type (of, name, t);
-        }
+	{
+	  struct type *t
+	    = ctf_init_float_type (of, cet.cte_bits / 2, NULL, name);
+	  type = init_complex_type (of, name, t);
+	}
     }
   else
     {
@@ -540,7 +536,7 @@  read_base_type (ctf_context_t *ccp, ctf_id_t tid)
       type = init_type (of, TYPE_CODE_ERROR, cet.cte_bits, name);
     }
 
-  if (name && strcmp (name, "char") == 0)
+  if (name != NULL && strcmp (name, "char") == 0)
     TYPE_NOSIGN (type) = 1;
 
   return set_tid_type (of, tid, type);
@@ -555,8 +551,8 @@  process_base_type (ctf_context_t *ccp, ctf_id_t tid)
   new_symbol (ccp, type, tid);
 }
 
-/* Start a structure or union scope (definition) with TID and TP to create
-   a type for the structure or union.
+/* Start a structure or union scope (definition) with TID to create a type
+   for the structure or union.
 
    Fill in the type's name and general properties. The members will not be
    processed, nor a symbol table entry be done until process_structure_type
@@ -568,22 +564,24 @@  read_structure_type (ctf_context_t *ccp, ctf_id_t tid)
   struct objfile *of = ccp->of;
   ctf_file_t *fp = ccp->fp;
   struct type *type;
-  const char *name;
+  char *name;
   uint32_t kind;
 
   type = alloc_type (of);
+
   name = ctf_type_aname_raw (fp, tid);
-  if (name && strlen (name))
-    TYPE_NAME (type) = name;
-  kind = ctf_type_kind (fp, tid);
-  if (kind == CTF_K_UNION)
+  if (name != NULL && strlen (name) != 0)
     {
-      TYPE_CODE (type) = TYPE_CODE_UNION;
+      TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name);
+      free (name);
     }
+
+  kind = ctf_type_kind (fp, tid);
+  if (kind == CTF_K_UNION)
+    TYPE_CODE (type) = TYPE_CODE_UNION;
   else
-    {
-      TYPE_CODE (type) = TYPE_CODE_STRUCT;
-    }
+    TYPE_CODE (type) = TYPE_CODE_STRUCT;
+
   TYPE_LENGTH (type) = ctf_type_size (fp, tid);
   set_type_align (type, ctf_type_align (fp, tid));
 
@@ -595,20 +593,15 @@  read_structure_type (ctf_context_t *ccp, ctf_id_t tid)
 
 static void
 process_struct_members (ctf_context_t *ccp,
-                        ctf_id_t tid,
-                        struct type *type)
+			ctf_id_t tid,
+			struct type *type)
 {
-  ctf_file_t *fp = ccp->fp;
   struct field_info fi;
 
-  fi.cur_context.fp = fp;
-  fi.cur_context.of = ccp->of;
-  fi.cur_context.builder = ccp->builder;
-  if (ctf_member_iter (fp, tid, ctf_add_member_cb, &fi) == CTF_ERR)
-    {
-      complaint (_("ctf_member_iter process_struct_members failed - %s"),
-                 ctf_errmsg (ctf_errno (fp)));
-    }
+  fi.cur_context = ccp;
+  if (ctf_member_iter (ccp->fp, tid, ctf_add_member_cb, &fi) == CTF_ERR)
+    complaint (_("ctf_member_iter process_struct_members failed - %s"),
+	       ctf_errmsg (ctf_errno (ccp->fp)));
 
   /* Attach fields to the type.  */
   attach_fields_to_type (&fi, type);
@@ -617,8 +610,7 @@  process_struct_members (ctf_context_t *ccp,
 }
 
 static void
-process_structure_type (ctf_context_t *ccp,
-                        ctf_id_t tid)
+process_structure_type (ctf_context_t *ccp, ctf_id_t tid)
 {
   struct type *type;
 
@@ -626,7 +618,7 @@  process_structure_type (ctf_context_t *ccp,
   process_struct_members (ccp, tid, type);
 }
 
-/* Create a function type for TID, set its return type using TP.  */
+/* Create a function type for TID and set its return type.  */
 
 static struct type *
 read_func_kind_type (ctf_context_t *ccp, ctf_id_t tid)
@@ -635,12 +627,17 @@  read_func_kind_type (ctf_context_t *ccp, ctf_id_t tid)
   ctf_file_t *fp = ccp->fp;
   struct type *type, *rettype;
   ctf_funcinfo_t cfi;
-  const char *name;
+  char *name;
 
   type = alloc_type (objfile);
+
   name = ctf_type_aname_raw (fp, tid);
-  if (name && strlen (name))
-    TYPE_NAME (type) = name;
+  if (name != NULL && strlen (name) != 0)
+    {
+      TYPE_NAME (type) = obstack_strdup (&objfile->objfile_obstack, name);
+      free (name);
+    }
+
   TYPE_CODE (type) = TYPE_CODE_FUNC;
   ctf_func_type_info (fp, tid, &cfi);
   rettype = get_tid_type (objfile, cfi.ctc_return);
@@ -650,23 +647,27 @@  read_func_kind_type (ctf_context_t *ccp, ctf_id_t tid)
   return set_tid_type (objfile, tid, type);
 }
 
-/* Given a TID of CTF_K_ENUM with type node TP, process all the members of
-   the enumeration, and create the symbol for the enumeration type.  */
+/* Given a TID of CTF_K_ENUM, process all the members of the
+   enumeration, and create the symbol for the enumeration type.  */
 
 static struct type *
-read_enum_type (ctf_context_t *ccp,
-                       ctf_id_t tid)
+read_enum_type (ctf_context_t *ccp, ctf_id_t tid)
 {
   struct objfile *of = ccp->of;
   ctf_file_t *fp = ccp->fp;
   struct type *type, *target_type;
   ctf_funcinfo_t fi;
-  const char *name;
+  char *name;
 
   type = alloc_type (of);
+
   name = ctf_type_aname_raw (fp, tid);
-  if (name && strlen (name))
-    TYPE_NAME (type) = name;
+  if (name != NULL && strlen (name) != 0)
+    {
+      TYPE_NAME (type) = obstack_strdup (&of->objfile_obstack, name);
+      free (name);
+    }
+
   TYPE_CODE (type) = TYPE_CODE_ENUM;
   TYPE_LENGTH (type) = ctf_type_size (fp, tid);
   ctf_func_type_info (fp, tid, &fi);
@@ -678,24 +679,18 @@  read_enum_type (ctf_context_t *ccp,
 }
 
 static void
-process_enum_type (ctf_context_t *ccp,
-                   ctf_id_t tid)
+process_enum_type (ctf_context_t *ccp, ctf_id_t tid)
 {
-  ctf_file_t *fp = ccp->fp;
   struct type *type;
   struct field_info fi;
 
   type = read_enum_type (ccp, tid);
 
-  fi.cur_context.fp = fp;
-  fi.cur_context.of = ccp->of;
-  fi.cur_context.builder = ccp->builder;
+  fi.cur_context = ccp;
   fi.ptype = type;
-  if (ctf_enum_iter (fp, tid, ctf_add_enum_member_cb, &fi) == CTF_ERR)
-    {
-      complaint (_("ctf_enum_iter process_enum_type failed - %s"),
-                 ctf_errmsg (ctf_errno (fp)));
-    }
+  if (ctf_enum_iter (ccp->fp, tid, ctf_add_enum_member_cb, &fi) == CTF_ERR)
+    complaint (_("ctf_enum_iter process_enum_type failed - %s"),
+	       ctf_errmsg (ctf_errno (ccp->fp)));
 
   /* Attach fields to the type.  */
   attach_fields_to_type (&fi, type);
@@ -707,10 +702,10 @@  process_enum_type (ctf_context_t *ccp,
 
 static struct type *
 add_array_cv_type (ctf_context_t *ccp,
-                   ctf_id_t tid,
-                   struct type *base_type,
-                   int cnst,
-                   int voltl)
+		   ctf_id_t tid,
+		   struct type *base_type,
+		   int cnst,
+		   int voltl)
 {
   struct type *el_type, *inner_array;
 
@@ -719,8 +714,8 @@  add_array_cv_type (ctf_context_t *ccp,
 
   while (TYPE_CODE (TYPE_TARGET_TYPE (inner_array)) == TYPE_CODE_ARRAY)
     {
-      TYPE_TARGET_TYPE (inner_array) =
-	copy_type (TYPE_TARGET_TYPE (inner_array));
+      TYPE_TARGET_TYPE (inner_array)
+	= copy_type (TYPE_TARGET_TYPE (inner_array));
       inner_array = TYPE_TARGET_TYPE (inner_array);
     }
 
@@ -732,7 +727,7 @@  add_array_cv_type (ctf_context_t *ccp,
   return set_tid_type (ccp->of, tid, base_type);
 }
 
-/* Read all information from a CTF_K_ARRAY TID with type node TP.  */
+/* Read all information from a TID of CTF_K_ARRAY.  */
 
 static struct type *
 read_array_type (ctf_context_t *ccp, ctf_id_t tid)
@@ -746,17 +741,19 @@  read_array_type (ctf_context_t *ccp, ctf_id_t tid)
   if (ctf_array_info (fp, tid, &ar) == CTF_ERR)
     {
       complaint (_("ctf_array_info read_array_type failed - %s"),
-                 ctf_errmsg (ctf_errno (fp)));
+		 ctf_errmsg (ctf_errno (fp)));
       return NULL;
     }
+
   element_type = get_tid_type (objfile, ar.ctr_contents);
-  if (!element_type)
+  if (element_type == NULL)
     return NULL;
+
   idx_type = get_tid_type (objfile, ar.ctr_index);
-  if (!idx_type)
+  if (idx_type == NULL)
     idx_type = objfile_type (objfile)->builtin_int;
 
-  range_type = create_static_range_type (NULL, idx_type, 0, ar.ctr_nelems-1);
+  range_type = create_static_range_type (NULL, idx_type, 0, ar.ctr_nelems - 1);
   type = create_array_type (NULL, element_type, range_type);
   if (ar.ctr_nelems <= 1)	/* Check if undefined upper bound.  */
     {
@@ -765,9 +762,8 @@  read_array_type (ctf_context_t *ccp, ctf_id_t tid)
       TYPE_TARGET_STUB (type) = 1;
     }
   else
-    {
-      TYPE_LENGTH (type) = ctf_type_size (fp, tid);
-    }
+    TYPE_LENGTH (type) = ctf_type_size (fp, tid);
+
   set_type_align (type, ctf_type_align (fp, tid));
 
   return set_tid_type (objfile, tid, type);
@@ -785,11 +781,11 @@  read_const_type (ctf_context_t *ccp, ctf_id_t tid, ctf_id_t btid)
   if (base_type == NULL)
     {
       base_type = read_type_record (ccp, btid);
-      if (!base_type)
-        {
-          complaint (_("read_const_type: NULL base type (%ld)"), btid);
+      if (base_type == NULL)
+	{
+	  complaint (_("read_const_type: NULL base type (%ld)"), btid);
 	  base_type = objfile_type (objfile)->builtin_error;
-        }
+	}
     }
   cv_type = make_cv_type (1, TYPE_VOLATILE (base_type), base_type, 0);
 
@@ -806,10 +802,14 @@  read_volatile_type (ctf_context_t *ccp, ctf_id_t tid, ctf_id_t btid)
   struct type *base_type, *cv_type;
 
   base_type = get_tid_type (objfile, btid);
-  if (!base_type)
+  if (base_type == NULL)
     {
-      complaint (_("read_volatile_type: NULL base type (%ld)"), btid);
-      base_type = objfile_type (objfile)->builtin_error;
+      base_type = read_type_record (ccp, btid);
+      if (base_type == NULL)
+	{
+	  complaint (_("read_volatile_type: NULL base type (%ld)"), btid);
+	  base_type = objfile_type (objfile)->builtin_error;
+	}
     }
 
   if (ctf_type_kind (fp, btid) == CTF_K_ARRAY)
@@ -831,11 +831,11 @@  read_restrict_type (ctf_context_t *ccp, ctf_id_t tid, ctf_id_t btid)
   if (base_type == NULL)
     {
       base_type = read_type_record (ccp, btid);
-      if (!base_type)
-        {
-          complaint (_("read_restrict_type: NULL base type (%ld)"), btid);
+      if (base_type == NULL)
+	{
+	  complaint (_("read_restrict_type: NULL base type (%ld)"), btid);
 	  base_type = objfile_type (objfile)->builtin_error;
-        }
+	}
     }
   cv_type = make_restrict_type (base_type);
 
@@ -846,19 +846,20 @@  read_restrict_type (ctf_context_t *ccp, ctf_id_t tid, ctf_id_t btid)
 
 static struct type *
 read_typedef_type (ctf_context_t *ccp, ctf_id_t tid,
-                   ctf_id_t btid, const char *name)
+		   ctf_id_t btid, const char *name)
 {
   struct objfile *objfile = ccp->of;
   struct type *this_type, *target_type;
 
-  this_type = init_type (objfile, TYPE_CODE_TYPEDEF, 0, name);
+  char *aname = obstack_strdup (&objfile->objfile_obstack, name);
+  this_type = init_type (objfile, TYPE_CODE_TYPEDEF, 0, aname);
   set_tid_type (objfile, tid, this_type);
   target_type = get_tid_type (objfile, btid);
   if (target_type != this_type)
     TYPE_TARGET_TYPE (this_type) = target_type;
   else
     TYPE_TARGET_TYPE (this_type) = NULL;
-  TYPE_TARGET_STUB (this_type) = TYPE_TARGET_TYPE (this_type)? 1 : 0;
+  TYPE_TARGET_STUB (this_type) = TYPE_TARGET_TYPE (this_type) ? 1 : 0;
 
   return set_tid_type (objfile, tid, this_type);
 }
@@ -872,14 +873,14 @@  read_pointer_type (ctf_context_t *ccp, ctf_id_t tid, ctf_id_t btid)
   struct type *target_type, *type;
 
   target_type = get_tid_type (of, btid);
-  if (!target_type)
+  if (target_type == NULL)
     {
       target_type = read_type_record (ccp, btid);
-      if (!target_type)
-        {
-          complaint (_("read_pointer_type: NULL target type (%ld)"), btid);
+      if (target_type == NULL)
+	{
+	  complaint (_("read_pointer_type: NULL target type (%ld)"), btid);
 	  target_type = objfile_type (ccp->of)->builtin_error;
-        }
+	}
     }
 
   type = lookup_pointer_type (target_type);
@@ -894,7 +895,7 @@  static struct type *
 read_type_record (ctf_context_t *ccp, ctf_id_t tid)
 {
   ctf_file_t *fp = ccp->fp;
-  const char *name;
+  char *name;
   uint32_t kind;
   struct type *type = NULL;
   ctf_id_t btid;
@@ -908,42 +909,43 @@  read_type_record (ctf_context_t *ccp, ctf_id_t tid)
 	break;
       case CTF_K_ENUM:
 	type = read_enum_type (ccp, tid);
-        break;
+	break;
       case CTF_K_FUNCTION:
 	type = read_func_kind_type (ccp, tid);
-        break;
+	break;
       case CTF_K_CONST:
-        btid = ctf_type_reference (fp, tid);
+	btid = ctf_type_reference (fp, tid);
 	type = read_const_type (ccp, tid, btid);
-        break;
+	break;
       case CTF_K_TYPEDEF:
-        name = ctf_type_aname_raw (fp, tid);
-        btid = ctf_type_reference (fp, tid);
+	name = ctf_type_aname_raw (fp, tid);
+	btid = ctf_type_reference (fp, tid);
 	type = read_typedef_type (ccp, tid, btid, name);
-        break;
+	free (name);
+	break;
       case CTF_K_VOLATILE:
-        btid = ctf_type_reference (fp, tid);
+	btid = ctf_type_reference (fp, tid);
 	type = read_volatile_type (ccp, tid, btid);
-        break;
+	break;
       case CTF_K_RESTRICT:
-        btid = ctf_type_reference (fp, tid);
+	btid = ctf_type_reference (fp, tid);
 	type = read_restrict_type (ccp, tid, btid);
-        break;
+	break;
       case CTF_K_POINTER:
-        btid = ctf_type_reference (fp, tid);
+	btid = ctf_type_reference (fp, tid);
 	type = read_pointer_type (ccp, tid, btid);
-        break;
+	break;
       case CTF_K_INTEGER:
       case CTF_K_FLOAT:
 	type = read_base_type (ccp, tid);
-        break;
+	break;
       case CTF_K_ARRAY:
 	type = read_array_type (ccp, tid);
-        break;
+	break;
       case CTF_K_UNKNOWN:
-        break;
+	break;
       default:
-        break;
+	break;
     }
 
   return type;
@@ -960,7 +962,7 @@  ctf_add_type_cb (ctf_id_t tid, void *arg)
 
   /* Check if tid's type has already been defined.  */
   type = get_tid_type (ccp->of, tid);
-  if (type)
+  if (type != NULL)
     return 0;
 
   ctf_id_t btid = ctf_type_reference (ccp->fp, tid);
@@ -973,42 +975,42 @@  ctf_add_type_cb (ctf_id_t tid, void *arg)
 	break;
       case CTF_K_ENUM:
 	process_enum_type (ccp, tid);
-        break;
+	break;
       case CTF_K_FUNCTION:
 	type = read_func_kind_type (ccp, tid);
 	new_symbol (ccp, type, tid);
-        break;
+	break;
       case CTF_K_INTEGER:
       case CTF_K_FLOAT:
 	process_base_type (ccp, tid);
-        break;
+	break;
       case CTF_K_TYPEDEF:
 	new_symbol (ccp, read_type_record (ccp, tid), tid);
-        break;
+	break;
       case CTF_K_CONST:
 	type = read_const_type (ccp, tid, btid);
 	new_symbol (ccp, type, tid);
-        break;
+	break;
       case CTF_K_VOLATILE:
 	type = read_volatile_type (ccp, tid, btid);
 	new_symbol (ccp, type, tid);
-        break;
+	break;
       case CTF_K_RESTRICT:
 	type = read_restrict_type (ccp, tid, btid);
 	new_symbol (ccp, type, tid);
-        break;
+	break;
       case CTF_K_POINTER:
 	type = read_pointer_type (ccp, tid, btid);
 	new_symbol (ccp, type, tid);
-        break;
+	break;
       case CTF_K_ARRAY:
 	type = read_array_type (ccp, tid);
 	new_symbol (ccp, type, tid);
-        break;
+	break;
       case CTF_K_UNKNOWN:
-        break;
+	break;
       default:
-        break;
+	break;
     }
 
   return 0;
@@ -1031,8 +1033,8 @@  ctf_add_var_cb (const char *name, ctf_id_t id, void *arg)
     {
       case CTF_K_FUNCTION:
 	if (name && !strcmp(name, "main"))
-          set_objfile_main_name (ccp->of, name, language_c);
-        break;
+	  set_objfile_main_name (ccp->of, name, language_c);
+	break;
       case CTF_K_INTEGER:
       case CTF_K_FLOAT:
       case CTF_K_VOLATILE:
@@ -1041,47 +1043,44 @@  ctf_add_var_cb (const char *name, ctf_id_t id, void *arg)
       case CTF_K_CONST:
       case CTF_K_POINTER:
       case CTF_K_ARRAY:
-        if (type)
-    	  {
+	if (type)
+	  {
 	    sym = new_symbol (ccp, type, id);
-            SYMBOL_SET_NAMES (sym, name, strlen (name), 0, ccp->of);
-          }
-        break;
+	    SYMBOL_SET_NAMES (sym, name, strlen (name), 0, ccp->of);
+	  }
+	break;
       case CTF_K_STRUCT:
       case CTF_K_UNION:
       case CTF_K_ENUM:
-        if (!type)
-        {
-          complaint (_("ctf_add_var_cb: %s has NO type (%ld)"), name, id);
-          type = objfile_type (ccp->of)->builtin_error;
-        }
-        sym = allocate_symbol (ccp->of);
-        OBJSTAT (ccp->of, n_syms++);
+	if (type == NULL)
+	{
+	  complaint (_("ctf_add_var_cb: %s has NO type (%ld)"), name, id);
+	  type = objfile_type (ccp->of)->builtin_error;
+	}
+	sym = allocate_symbol (ccp->of);
+	OBJSTAT (ccp->of, n_syms++);
 	SYMBOL_TYPE (sym) = type;
 	SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
-        SYMBOL_ACLASS_INDEX (sym) = LOC_OPTIMIZED_OUT;
-        SYMBOL_SET_NAMES (sym, name, strlen (name), 0, ccp->of);
-        add_symbol_to_list (sym, ccp->builder->get_global_symbols ());
-        break;
+	SYMBOL_ACLASS_INDEX (sym) = LOC_OPTIMIZED_OUT;
+	SYMBOL_SET_NAMES (sym, name, strlen (name), 0, ccp->of);
+	add_symbol_to_list (sym, ccp->builder->get_global_symbols ());
+	break;
       default:
-        complaint (_("ctf_add_var_cb: kind unsupported (%d)"), kind);
-        break;
+	complaint (_("ctf_add_var_cb: kind unsupported (%d)"), kind);
+	break;
     }
 
   if (sym)
-    {
-      set_symbol_address (ccp->of, sym, name);
-    }
+    set_symbol_address (ccp->of, sym, name);
 
   return 0;
 }
 
 /* Add an ELF STT_OBJ symbol with index IDX to the symbol table.  */
 
-struct symbol *
+static struct symbol *
 add_stt_obj (ctf_context_t *ccp, unsigned long idx)
 {
-  const char *name;
   struct symbol *sym;
   struct type *type;
   ctf_id_t tid;
@@ -1093,22 +1092,18 @@  add_stt_obj (ctf_context_t *ccp, unsigned long idx)
   if (type == NULL)
     return NULL;
 
-  name = ctf_type_aname_raw (ccp->fp, idx);
-  if (name == NULL)
-    return NULL;
   sym = new_symbol (ccp, type, tid);
-  SYMBOL_SET_NAMES (sym, name, strlen (name), 0, ccp->of);
 
   return sym;
 }
 
 /* Add an ELF STT_FUNC symbol with index IDX to the symbol table.  */
 
-struct symbol *
+static struct symbol *
 add_stt_func (ctf_context_t *ccp, unsigned long idx)
 {
   struct type *ftype, *atyp, *rettyp;
-  const char *name;
+  char *name;
   struct symbol *sym;
   ctf_funcinfo_t finfo;
   ctf_id_t argv[32];
@@ -1126,15 +1121,18 @@  add_stt_func (ctf_context_t *ccp, unsigned long idx)
   name = ctf_type_aname_raw (ccp->fp, idx);
   if (name == NULL)
     return NULL;
+  free (name);
+
   tid = ctf_lookup_by_symbol (ccp->fp, idx);
   ftype = get_tid_type (ccp->of, tid);
   if (finfo.ctc_flags & CTF_FUNC_VARARG)
     TYPE_VARARGS (ftype) = 1;
   TYPE_NFIELDS (ftype) = argc;
+
   /* If argc is 0, it has a "void" type.  */
-  if (argc)
-    TYPE_FIELDS (ftype) = (struct field *)
-      TYPE_ZALLOC (ftype, argc * sizeof (struct field));
+  if (argc != 0)
+    TYPE_FIELDS (ftype)
+      = (struct field *) TYPE_ZALLOC (ftype, argc * sizeof (struct field));
 
   /* TYPE_FIELD_TYPE must never be NULL.  Fill it with void_type, if failed
      to find the argument type.  */
@@ -1142,9 +1140,9 @@  add_stt_func (ctf_context_t *ccp, unsigned long idx)
     {
       atyp = get_tid_type (ccp->of, argv[iparam]);
       if (atyp)
-    	TYPE_FIELD_TYPE (ftype, iparam) = atyp;
+	TYPE_FIELD_TYPE (ftype, iparam) = atyp;
       else
-    	TYPE_FIELD_TYPE (ftype, iparam) = void_type;
+	TYPE_FIELD_TYPE (ftype, iparam) = void_type;
     }
 
   sym = new_symbol (ccp, ftype, tid);
@@ -1153,7 +1151,6 @@  add_stt_func (ctf_context_t *ccp, unsigned long idx)
     SYMBOL_TYPE (sym) = rettyp;
   else
     SYMBOL_TYPE (sym) = void_type;
-  SYMBOL_SET_NAMES (sym, name, strlen (name), 0, ccp->of);
 
   return sym;
 }
@@ -1161,14 +1158,14 @@  add_stt_func (ctf_context_t *ccp, unsigned long idx)
 /* Get text segment base for OBJFILE, TSIZE contains the segment size.  */
 
 static CORE_ADDR
-get_of_text_range (struct objfile *of, int *tsize)
+get_objfile_text_range (struct objfile *of, int *tsize)
 {
   CORE_ADDR text_base;
   bfd *abfd = of->obfd;
   const asection *codes;
 
   codes = bfd_get_section_by_name (abfd, ".text");
-  if (!codes)
+  if (codes == NULL)
     {
       text_base = 0;
       *tsize = 0;
@@ -1186,14 +1183,14 @@  get_of_text_range (struct objfile *of, int *tsize)
 
 static void
 ctf_start_symtab (struct partial_symtab *pst,
-                  struct objfile *of, CORE_ADDR text_offset)
+		  struct objfile *of, CORE_ADDR text_offset)
 {
   ctf_context_t *ccp;
 
   ccp = (ctf_context_t *) pst->read_symtab_private;
-  ccp->builder = new struct buildsym_compunit
+  ccp->builder = new buildsym_compunit
 		       (of, of->original_name, NULL,
-	               language_c, text_offset);
+		       language_c, text_offset);
   ccp->builder->record_debugformat ("ctf");
 }
 
@@ -1203,7 +1200,7 @@  ctf_start_symtab (struct partial_symtab *pst,
 
 static struct compunit_symtab *
 ctf_end_symtab (struct partial_symtab *pst,
-                CORE_ADDR end_addr, int section)
+		CORE_ADDR end_addr, int section)
 {
   ctf_context_t *ccp;
 
@@ -1223,38 +1220,33 @@  psymtab_to_symtab (struct partial_symtab *pst)
   struct symbol *sym;
   ctf_context_t *ccp;
 
-  if (pst->readin == 1)
-    return;
+  gdb_assert (!pst->readin);
 
   ccp = (ctf_context_t *) pst->read_symtab_private;
 
   /* Iterate over entries in data types section.  */
   if (ctf_type_iter (ccp->fp, ctf_add_type_cb, ccp) == CTF_ERR)
-    {
-      complaint (_("ctf_type_iter psymtab_to_symtab failed - %s"),
-                 ctf_errmsg (ctf_errno (ccp->fp)));
-    }
+    complaint (_("ctf_type_iter psymtab_to_symtab failed - %s"),
+	       ctf_errmsg (ctf_errno (ccp->fp)));
 
 
   /* Iterate over entries in variable info section.  */
   if (ctf_variable_iter (ccp->fp, ctf_add_var_cb, ccp) == CTF_ERR)
-    {
-      complaint (_("ctf_variable_iter psymtab_to_symtab failed - %s"),
-                 ctf_errmsg (ctf_errno (ccp->fp)));
-    }
+    complaint (_("ctf_variable_iter psymtab_to_symtab failed - %s"),
+	       ctf_errmsg (ctf_errno (ccp->fp)));
 
   /* Add entries in data objects and function info sections.  */
   for (unsigned long i = 0; ; i++)
     {
       sym = add_stt_obj (ccp, i);
-      if (!sym)
-        {
+      if (sym == NULL)
+	{
 	  if (ctf_errno (ccp->fp) == EINVAL
-             || ctf_errno (ccp->fp) == ECTF_NOSYMTAB)
+	      || ctf_errno (ccp->fp) == ECTF_NOSYMTAB)
 	    break;
           sym = add_stt_func (ccp, i);
-        }
-      if (!sym)
+	}
+      if (sym == NULL)
 	continue;
 
       set_symbol_address (ccp->of, sym, SYMBOL_LINKAGE_NAME (sym));
@@ -1267,20 +1259,15 @@  psymtab_to_symtab (struct partial_symtab *pst)
    PST is not NULL.  */
 
 static void
-ctf_read_symtab (struct partial_symtab *pst,
-                 struct objfile *objfile)
+ctf_read_symtab (struct partial_symtab *pst, struct objfile *objfile)
 {
   if (pst->readin)
-    {
-      warning (_("bug: psymtab for %s is already read in."),
-	       pst->filename);
-    }
+    warning (_("bug: psymtab for %s is already read in."), pst->filename);
   else
     {
       if (info_verbose)
 	{
-	  printf_filtered (_("Reading in CTF data for %s..."),
-			   pst->filename);
+	  printf_filtered (_("Reading in CTF data for %s..."), pst->filename);
 	  gdb_flush (gdb_stdout);
 	}
 
@@ -1288,14 +1275,14 @@  ctf_read_symtab (struct partial_symtab *pst,
       CORE_ADDR text_offset;        /* Start of text segment.  */
       int tsize;
 
-      text_offset = get_of_text_range (objfile, &tsize);
+      text_offset = get_objfile_text_range (objfile, &tsize);
       ctf_start_symtab (pst, objfile, text_offset);
       psymtab_to_symtab (pst);
 
       pst->set_text_low (text_offset);
-      pst->set_text_high (text_offset+tsize);
+      pst->set_text_high (text_offset + tsize);
       pst->compunit_symtab = ctf_end_symtab (pst, text_offset + tsize,
-                                         SECT_OFF_TEXT (objfile));
+					     SECT_OFF_TEXT (objfile));
 
       /* Finish up the debug error message.  */
       if (info_verbose)
@@ -1314,8 +1301,9 @@  ctf_close_objfile (struct objfile *of, void *datum)
   ctf_close (arc);
 }
 
-/* Allocate a new partial_symtab NAME.  */
-/* Each source file that has not been fully read in is represented by
+/* Allocate a new partial_symtab NAME.
+
+   Each source file that has not been fully read in is represented by
    a partial_symtab.  This contains the information on where in the
    executable the debugging symbols for a specific file are, and a
    list of names of global symbols which are located in this file.
@@ -1327,8 +1315,8 @@  ctf_close_objfile (struct objfile *of, void *datum)
 
 static struct partial_symtab *
 create_partial_symtab (const char *name,
-                       ctf_file_t *cfp,
-                       struct objfile *objfile)
+		       ctf_file_t *cfp,
+		       struct objfile *objfile)
 {
   struct partial_symtab *pst;
   static ctf_context_t ccx;
@@ -1337,7 +1325,7 @@  create_partial_symtab (const char *name,
 
   ccx.fp = cfp;
   ccx.of = objfile;
-  pst->read_symtab_private = (void *)&ccx;
+  pst->read_symtab_private = (void *) &ccx;
   pst->read_symtab = ctf_read_symtab;
 
   return pst;
@@ -1349,13 +1337,13 @@  static int
 ctf_psymtab_type_cb (ctf_id_t tid, void *arg)
 {
   ctf_context_t *ccp;
-  const char *name;
+  char *name;
   uint32_t kind;
   short section = -1;
 
-  ccp = (ctf_context_t *)arg;
+  ccp = (ctf_context_t *) arg;
   name = ctf_type_aname_raw (ccp->fp, tid);
-  if (!name || (name && !strlen (name)))
+  if (name == NULL || strlen (name) == 0)
     return 0;
 
   domain_enum domain = UNDEF_DOMAIN;
@@ -1366,41 +1354,42 @@  ctf_psymtab_type_cb (ctf_id_t tid, void *arg)
       case CTF_K_STRUCT:
       case CTF_K_UNION:
       case CTF_K_ENUM:
-      	domain = STRUCT_DOMAIN;
-      	aclass = LOC_TYPEDEF;
-        break;
+	domain = STRUCT_DOMAIN;
+	aclass = LOC_TYPEDEF;
+	break;
       case CTF_K_FUNCTION:
       case CTF_K_FORWARD:
-      	domain = VAR_DOMAIN;
-      	aclass = LOC_STATIC;
-        section = SECT_OFF_TEXT (ccp->of);
-        break;
+	domain = VAR_DOMAIN;
+	aclass = LOC_STATIC;
+	section = SECT_OFF_TEXT (ccp->of);
+	break;
       case CTF_K_CONST:
-      	domain = VAR_DOMAIN;
-      	aclass = LOC_STATIC;
-        break;
+	domain = VAR_DOMAIN;
+	aclass = LOC_STATIC;
+	break;
       case CTF_K_TYPEDEF:
       case CTF_K_POINTER:
       case CTF_K_VOLATILE:
       case CTF_K_RESTRICT:
-      	domain = VAR_DOMAIN;
-      	aclass = LOC_TYPEDEF;
-        break;
+	domain = VAR_DOMAIN;
+	aclass = LOC_TYPEDEF;
+	break;
       case CTF_K_INTEGER:
       case CTF_K_FLOAT:
-      	domain = VAR_DOMAIN;
-      	aclass = LOC_TYPEDEF;
-        break;
+	domain = VAR_DOMAIN;
+	aclass = LOC_TYPEDEF;
+	break;
       case CTF_K_ARRAY:
       case CTF_K_UNKNOWN:
-        return 0;
+	return 0;
     }
 
-    add_psymbol_to_list (name, strlen (name), 1,
-                         domain, aclass, section,
-                         psymbol_placement::GLOBAL,
-                         0, language_c, ccp->of);
+    add_psymbol_to_list (name, strlen (name), true,
+			 domain, aclass, section,
+			 psymbol_placement::GLOBAL,
+			 0, language_c, ccp->of);
 
+  free (name);
   return 0;
 }
 
@@ -1411,10 +1400,10 @@  ctf_psymtab_var_cb (const char *name, ctf_id_t id, void *arg)
 {
   ctf_context_t *ccp = (ctf_context_t *) arg;
 
-  add_psymbol_to_list (name, strlen (name), 1,
-                       VAR_DOMAIN, LOC_STATIC, -1,
-                       psymbol_placement::GLOBAL,
-                       0, language_c, ccp->of);
+  add_psymbol_to_list (name, strlen (name), true,
+		       VAR_DOMAIN, LOC_STATIC, -1,
+		       psymbol_placement::GLOBAL,
+		       0, language_c, ccp->of);
   return 0;
 }
 
@@ -1431,17 +1420,14 @@  scan_partial_symbols (ctf_file_t *cfp, struct objfile *of)
 
   ccx.fp = cfp;
   ccx.of = of;
+
   if (ctf_type_iter (cfp, ctf_psymtab_type_cb, &ccx) == CTF_ERR)
-    {
-      complaint (_("ctf_type_iter scan_partial_symbols failed - %s"),
-                 ctf_errmsg (ctf_errno (cfp)));
-    }
+    complaint (_("ctf_type_iter scan_partial_symbols failed - %s"),
+	       ctf_errmsg (ctf_errno (cfp)));
 
   if (ctf_variable_iter (cfp, ctf_psymtab_var_cb, &ccx) == CTF_ERR)
-    {
-      complaint (_("ctf_variable_iter scan_partial_symbols failed - %s"),
-                 ctf_errmsg (ctf_errno (cfp)));
-    }
+    complaint (_("ctf_variable_iter scan_partial_symbols failed - %s"),
+	       ctf_errmsg (ctf_errno (cfp)));
 
   /* Scan CTF object and function sections which correspond to each
      STT_FUNC or STT_OBJECT entry in the symbol table,
@@ -1450,41 +1436,40 @@  scan_partial_symbols (ctf_file_t *cfp, struct objfile *of)
     {
       ctf_id_t tid;
       if ((tid = ctf_lookup_by_symbol (cfp, idx)) == CTF_ERR)
-        {
-	if (ctf_errno (cfp) == EINVAL
-           || ctf_errno (cfp) == ECTF_NOSYMTAB)
-	    // case ECTF_SYMRANGE:
-	  break;
+	{
+	if (ctf_errno (cfp) == EINVAL || ctf_errno (cfp) == ECTF_NOSYMTAB)
+	  break;	// Done, reach end of the section.
 	else
 	  continue;
-        }
-      const char *tname = ctf_type_aname_raw (cfp, tid);
+	}
+      char *tname = ctf_type_aname_raw (cfp, tid);
       uint32_t kind = ctf_type_kind (cfp, tid);
       address_class aclass;
       domain_enum tdomain;
       switch (kind)
-        {
-          case CTF_K_STRUCT:
-          case CTF_K_UNION:
-          case CTF_K_ENUM:
+	{
+	  case CTF_K_STRUCT:
+	  case CTF_K_UNION:
+	  case CTF_K_ENUM:
 	    tdomain = STRUCT_DOMAIN;
 	    break;
-          default:
+	  default:
 	    tdomain = VAR_DOMAIN;
 	    break;
-        }
+	}
 
       if (kind == CTF_K_FUNCTION)
-        aclass = LOC_STATIC;
+	aclass = LOC_STATIC;
       else if (kind == CTF_K_CONST)
-        aclass = LOC_CONST;
+	aclass = LOC_CONST;
       else
-        aclass = LOC_TYPEDEF;
+	aclass = LOC_TYPEDEF;
 
-      add_psymbol_to_list (tname, strlen (name), 1,
-                           tdomain, aclass, -1,
-                           psymbol_placement::STATIC,
-                           0, language_c, of);
+      add_psymbol_to_list (tname, strlen (tname), true,
+			   tdomain, aclass, -1,
+			   psymbol_placement::STATIC,
+			   0, language_c, of);
+      free (tname);
     }
 
   end_psymtab_common (of, pst);
@@ -1501,14 +1486,14 @@  elfctf_build_psymtabs (struct objfile *of)
   int err;
 
   ctf_archive_t *arc = ctf_bfdopen (abfd, &err);
-  if (!arc)
+  if (arc == NULL)
     error (_("ctf_bfdopen failed on %s - %s"),
-           bfd_get_filename (abfd), ctf_errmsg (err));
+	   bfd_get_filename (abfd), ctf_errmsg (err));
 
   ctf_file_t *fp = ctf_arc_open_by_name (arc, NULL, &err);
-  if (!fp)
+  if (fp == NULL)
     error (_("ctf_arc_open_by_name failed on %s - %s"),
-           bfd_get_filename (abfd), ctf_errmsg (err));
+	   bfd_get_filename (abfd), ctf_errmsg (err));
   set_objfile_data (of, ctf_file_key, fp);
 
   scan_partial_symbols (fp, of);