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

[Michael Snyder <msnyder@vmware.com>]

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.   ;-)