This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


> 2010-03-04  Hui Zhu  <teawater@gmail.com>
> 
> 	* i386-tdep.c (i386_process_record): Change "addr" to "tmpu64".

OK.

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.

-- 
Joel


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]