Re: [RFA, 3 of 3] save/restore process record, part 3 (save/restore)

Michael Snyder
2009-10-20T20:03:30Z

Hui Zhu wrote:
> On Tue, Oct 20, 2009 at 01:54, Michael Snyder <msnyder@vmware.com> wrote:
>> Hui Zhu wrote:
>>> Hi Michael,
>>>
>>> + do_cleanups (old_cleanups);
>>>
>>> This line will remove the record file that we just save.
>>>
>>> I change some code:
>>> static void
>>> record_save_cleanups (void *data)
>>> {
>>> bfd *obfd = data;
>>> //char *pathname = xstrdup (bfd_get_filename (obfd));
>>> bfd_close (obfd);
>>> //unlink (pathname);
>>> //xfree (pathname);
>>> }
>>>
>>> I think you want unlink the gdb_record when save get some error. It
>>> maybe need "discard_cleanups" the old_cleanups and bfd_close (obfd);
>>>
>>> After change the code, everything is OK.
>> Yes. Thanks. Like this:
>> + if (record_list->prev)
>> + record_list = record_list->prev;
>> + }
>> +
>> + do_cleanups (set_cleanups);
>> + bfd_close (obfd);
>> + discard_cleanups (old_cleanups);
>> +
>> + /* Succeeded. */
>
>
> I suggest:
> + discard_cleanups (old_cleanups);
> + bfd_close (obfd);

The reason I did the bfd_close first is because I wasn't
sure if it was safe to delete the file first. In any way,
it seems more logical to close the file before delete it.

What do you think?

> BTW, the record.c update by Pedro,
> http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/record.c.diff?r1=1.24&r2=1.25&cvsroot=src
>
> Maybe the record save patch need some update.

Hmmm. Yes, I'm sure it will. ;-)