Re: [RFA] Submit process record and replay third time, 3/9

[Pedro Alves <pedro@codesourcery.com>]

On Wednesday 04 March 2009 03:41:59, teawater wrote:

> >> +# For the process record and replay target.
> >> +M:int:process_record:CORE_ADDR addr:addr
> >
> > You'll need to extend this comment a little further.  What is this
> > callback really for?  E.g., what is it supposed to do?  These things
> > should be documented here.
> 
> What about the following:
> # Record a execution log of instruction at address addr.

How about something more descriptive like:

/* Parse the instruction at ADDR storing in the record execution log
   the registers and memory ranges that will be affected when the
   instruction executes, along with their current values.  Return -1
   if something goes wrong, 0 otherwise.  */

> > About the interface itself, would it be possible to adjust the
> > interface to make this callback's implementations not call record.c
> > functions, but instead have record.c work only with the results of
> > this callback?
> 
> Are you mean i386_process_record doesn't call the function in record.c?

Yes.

> That is so hard.  A lot of this record is same for each arch.  So I
> encapsulation them to be some function.
> For example,  record_arch_list_add_reg and record_arch_list_add_mem.
> Another arch will need it in the future.
> So, do you think it's ok?

Let's keep it as is for now.  Not much use in iterating over
this detail.

> >> +M:void:process_record_dasm:void
> >> +
> >
> > I'm puzzled by this one.  What's this for?  I can't see it being
> > used anywhere, did I miss something?  What's "dasm"?  If its not
> > used for anything yet, let's remove it for now.
> >
> In replay mode, gdb will call gdbarch_process_record_dasm to let arch
> special code analyzes the current instruction and do some replay job.
> It will make record speed up and decrease the memory use.
> 
> It just support by mips arch, but mips precord code is removed now.
> So I will removed it and add it back when some arch support it.

Thanks.

> By the way, process_record_dasm is so ugly name.  Do you have some idea with it?

No, sorry, because I don't know what "dasm" means.

-- 
Pedro Alves