Re: [PATCH] Remove gdb_bfd_stash_filename to fix crash with fix of binutils/11983

[Doug Evans <dje at google dot com>]

On Mon, Jan 6, 2014 at 2:50 AM, Hui Zhu <hui_zhu@mentor.com> wrote:
> On 01/06/14 16:25, Tom Tromey wrote:
>>>>>>>
>>>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:
>>
>>
>> Hui> Thanks.  Post a new version.
>>
>> Thanks Hui.  This is definitely the direction I think the code should
>> go.
>>
>> Hui>  --- a/gdb/symfile-mem.c
>> Hui> +++ b/gdb/symfile-mem.c
>> Hui> @@ -104,11 +104,7 @@ symbol_file_add_from_memory (struct bfd
>> Hui>    if (name == NULL)
>> Hui>  nbfd-> filename = "shared object read from target memory";
>> Hui>    else
>> Hui> -    {
>> Hui> -      nbfd->filename = name;
>> Hui> -      gdb_bfd_stash_filename (nbfd);
>> Hui> -      xfree (name);
>> Hui> -    }
>> Hui> +    nbfd->filename = name;
>> Hui>   cleanup = make_cleanup_bfd_unref (nbfd);
>>
>> In this hunk there are two things to note.
>>
>> First, there is an earlier assignment to filename (in the context above)
>> that should use xstrdup.
>>
>> Second, the new assignment really ought to free the old nbfd->filename
>> first.
>
>
> I changed this part to:
>   xfree (bfd_get_filename (nbfd));
>   if (name == NULL)
>     nbfd->filename = xstrdup ("shared object read from target memory");
>   else
>     nbfd->filename = name;

I would prefer a new bfd routine to set the file name.
Then *it* is responsible for freeing the old name.

Any reason to not go that route?

> --- a/gdb/symfile-mem.c
> +++ b/gdb/symfile-mem.c
> @@ -101,14 +101,11 @@ symbol_file_add_from_memory (struct bfd
>      error (_("Failed to read a valid object file image from memory."));
>     gdb_bfd_ref (nbfd);
> +  xfree (bfd_get_filename (nbfd));

This line still screams of excessive chumminess with bfd.

>    if (name == NULL)
> -    nbfd->filename = "shared object read from target memory";
> +    nbfd->filename = xstrdup ("shared object read from target memory");
>    else
> -    {
>
> -      nbfd->filename = name;
> -      gdb_bfd_stash_filename (nbfd);
> -      xfree (name);
> -    }
> +    nbfd->filename = name;
>     cleanup = make_cleanup_bfd_unref (nbfd);
>