Re: [RFA/RFC] Add dump and load command to process record and replay

[Hui Zhu <teawater at gmail dot com>]

About it, I think keep the old one can make the function more
flexible. I try it sometime.  It can make two or more gdb_record files
to one file.

What about add a warning to there? When the record list is not empty,
output a warning.

Thanks,
Hui

On Fri, Aug 7, 2009 at 11:04, Michael Snyder<msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>
>> +/* Load the execution log from a file. ?*/
>> +
>> +static void
>> +cmd_record_load (char *args, int from_tty)
>> +{
>> + ?int recfd;
>> + ?uint32_t magic;
>> + ?struct cleanup *old_cleanups;
>> + ?struct cleanup *old_cleanups2;
>> + ?struct record_entry *rec;
>> + ?int insn_number = 0;
>> +
>> + ?if (current_target.to_stratum != record_stratum)
>> + ? ?{
>> + ? ? ?cmd_record_start (NULL, from_tty);
>> + ? ? ?printf_unfiltered (_("Auto start process record.\n"));
>> + ? ?}
>> +
>> + ?if (!args || (args && !*args))
>> + ? ?error (_("Argument for filename required.\n"));
>> +
>> + ?/* Open the load file. ?*/
>> + ?recfd = open (args, O_RDONLY | O_BINARY);
>> + ?if (recfd < 0)
>> + ? ?error (_("Failed to open '%s' for loading execution records: %s"),
>> + ? ? ? ? ? args, strerror (errno));
>> + ?old_cleanups = make_cleanup (cmd_record_fd_cleanups, &recfd);
>> +
>> + ?/* Check the magic code. ?*/
>> + ?record_read_dump (args, recfd, &magic, 4);
>> + ?if (magic != RECORD_FILE_MAGIC)
>> + ? ?error (_("'%s' is not a valid dump of execution records."), args);
>> +
>> + ?/* Load the entries in recfd to the record_arch_list_head and
>> + ? ? record_arch_list_tail. ?*/
>> + ?record_arch_list_head = NULL;
>> + ?record_arch_list_tail = NULL;
>
> Hi Hui,
>
> It seems to me that you didn't discard the old recording entries
> before loading the new ones -- you just added the new ones into
> the existing list.
>
> I don't think that is safe. ?We can't be sure that there is
> any relationship between any existing recording entries and
> the new entries from the file. ?We certainly can't assume
> that they are consecutive.
>
> I think you have to completely clear the record log at this point,
> before you begin reading new entries from the dump file.
>
> What I would suggest is that we add these lines to
> record_list_release -- and then we can just call that here.
>
> @@ -159,6 +159,11 @@ record_list_release (struct record_entry
>
> ? if (rec != &record_first)
> ? ? xfree (rec);
> +
> + ?record_list = &record_first;
> + ?record_arch_list_tail = NULL;
> + ?record_arch_list_tail = NULL;
> + ?record_insn_num = 0;
> ?}
>
>
>