Re: [RFA] i386-tdep.c: fix a bug in prec i386 code

[Hui Zhu <teawater at gmail dot com>]

On Thu, Mar 4, 2010 at 15:38, Joel Brobecker <brobecker@adacore.com> wrote:
>
> > 2010-03-04 ?Hui Zhu ?<teawater@gmail.com>
> >
> > ? ? ? * i386-tdep.c (i386_process_record): Change "addr" to "tmpu64".
>
> OK.

Checked in.

>
> As an aside, your code needs a really good thorough cleanup. I warned
> you already about the use of variables with a meaningless name, and
> you'll make this sort of mistake again for as long as you keep using
> them. However, my main point is that the use of a giant switch statement
> makes your code very hard to read and review. ?I really suggest that
> you create a new file, precord-i386.c where you put your stuff there,
> and instead of inlining the code inside each case, you define small
> contained procedures for each instruction (or instruction group).
> It'll be easier to find the code that handles such and such instruction,
> easier to write a ChangeLog entry that tells us more about where the
> change was made, and it'll make the switch block actually possible
> to read.
>

Agree with you.
But prec patch have some still hang in there (see
http://sourceware.org/gdb/wiki/ProcessRecord) and this change will be
very big.
If I do it, what I will meet is:
1.  Make a patch follow the hang change, then the new patch cannot
checked in before the hang patch in.
2.  Make a patch just for cvs-head.  After this patch in, this hang
patch need update follow cvs-head.  This is not a small work.

I conflict with them.  So ...

Thanks,
Hui