Re: [PATCH] arm reversible : <phase_2_complete>

[paawan oza <paawan1982@yahoo.com>]

Hi Petr and Tom,

I have implemented most of your comments..
The comments which I could not put it in are below with explanation.

1) ARM_RECORD_ARCH_LIST_ADD_REG is used only at 2 places, but probably I wanted 
to 

give clear interface where it just takes only one argument, register number.
I should have done it for memory, but have not done yet.

sure, it hides local variable usage and it doesnt look good, but my intension 
was to give clear interface 

to function process_record modifiers.

I am still thinking over it.


2) > +   uint32_t opcode;              /* insn opcode.  */

I understand that above is one of the fields of arm_insn_record, but why I am 
usiong it as global field,
because the decoding bits by this field kind of global and common to most of the 
insns.

probably I would like to just extract it once and use anywhere, but 
unfortunately I have extracted in every function
and seems local, will work on that though.

3) 
+ start_address = start_address + 4
- start_address = start_address - 4
in two of the four cases.

If you have a look at the code, start_address initialization is the key factor 
for addressing mode.
not how start_address later gets incremented.. so even in decrement mode, 
increment is necessary but
initialization is different.

4)
In arm_handle_coproc_data_proc_insn():
> +            tdep->arm_swi_record(reg_cache);

When this line is executed this function still returns -1 (i.e. failure).
I guess the two if's are intended to return success then.

oza: the coprocessor insns are not implemented and syscall record too. so in 
both cases we return
-1.

5)
The return value is never tested. What is its meaning?

The parsed args seem to be stored as a side-effect. Where are they stored?
(Answers should go to comments.)
(Since this is an indirect call, the documentation should be more
verbose than usual cases.)

Oza: swi basically equivalent to sysenter an int80 insn on x86, so systemcall is 
implemented that way.
I am going to work on that implementation in phase 3.
phase 2 does not support it.

6) 
Tom: I still need to take care of endianess issue, which I will in the coming 
stage.

Note: updated patch will be mailed in the next mail.

Regards,
OZa.