Re: [PATCH 0/3] Minor refactorings in arm-tdep.c instruction decoding

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 16-02-11 06:17 AM, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
> 
>> I am currently working on extracting the instruction decoding from the
>> displaced stepping support in arm-tdep.c, in order to share the functionality
>> with the upcoming fast tracepoint support.  I did a few refactors that helped
>> me correlate the code with the ARM Architecture Reference Manual.  I think the
>> change helps readability in general, and especially when you have the manual
>> open on the side.
>>
>> The idea is to follow the the order of the manual, use the same names and do
>> the same "checks" (avoid using unnecessary shortcuts that make the code more
>> cryptic).
> 
> It is good to match the code to the manual.  The instruction encoding
> can't change, but the order or the category of instruction _may_ change in
> the manual in the future.  I am not the manual writer, but writers may
> want to refactor the doc in the future too :)
> 
> Although your change is code refactor, better to run tests.

Yes, that is the part that is worrying me.  Since we don't have unit tests,
I am too afraid to break things.

I'll forget this patch set for the time being.  When the code is extracted to
its own file and decoupled from the core of GDB, maybe there will be some way
to write some kind of unit tests that ensure that the refactor is safe.

Thanks for reviewing anyway!