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

[Hui Zhu <teawater@gmail.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;
>  }
>
>
>