[PATCH,V4 1/2] CTF: fix incorrect function return type

Wei-min Pan weimin.pan@oracle.com
Sun May 16 22:31:36 GMT 2021


On 5/7/2021 1:07 PM, Tom Tromey wrote:
>>>>>> ">" == Weimin Pan via Gdb-patches <gdb-patches@sourceware.org> writes:
>>> The problems can be illustrated, with any program, below:
>>> (gdb) print main
>>> $1 = {main} 0x0
> Hi.  Sorry about the delay in my re-review.
> It's fine (and expected) to send patch pings to the list.
> It helps keep us honest I guess.
>
>>> -      sym->compute_and_set_names (name.get (), true, objfile->per_bfd);
>>> +      sym->compute_and_set_names (name, true, objfile->per_bfd);
> If the lifetime of 'name' is ok, then you don't need to copy it here --
> so you can pass 'false' as the second parameter.  See the comment in
> symtab.c just before general_symbol_info::compute_and_set_names.
>
>>> +  name = ctf_type_name_raw (fp, tid);
>>> +  if (name == nullptr || strlen (name) == 0)
>>> +  {
> The braces (and the code they surround) look under-indented to me.
>
>>> -  ccp->pst->add_psymbol (name.get (), true,
>>> +  ccp->pst->add_psymbol (name, true,
> Same here.
>
>>> -      pst->add_psymbol (tname.get (), true,
>>> +      pst->add_psymbol (tname, true,
> And here.
>
> If you fix these up, and it still works, it is fine to push.
> If it doesn't work then that would tend to put some doubt on the other
> parts of the patch that don't copy names.

After making all the changes you suggested, plus changing 'true' to 
'false' in the
following two places to avoid name copying.

 > >> -  ccp->pst->add_psymbol (name.get (), true,
 > >> +  ccp->pst->add_psymbol (name, true,
 >
 > Same here.
 >
 > >> -      pst->add_psymbol (tname.get (), true,
 > >> +      pst->add_psymbol (tname, true,
 >
 > And here.

The CTF tests are passing. Just pushed.

Thanks.

>
> thanks,
> Tom


More information about the Gdb-patches mailing list