From: Joel Brobecker <brobecker@adacore.com>
To: Hui Zhu <teawater@gmail.com>
Cc: gdb-patches ml <gdb-patches@sourceware.org>,
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: [RFA] i386-tdep.c: fix a bug in prec i386 code
Date: Thu, 04 Mar 2010 07:39:00 -0000 [thread overview]
Message-ID: <20100304073855.GK2832@adacore.com> (raw)
In-Reply-To: <daef60381003031837r297e9bb2s70889676afb3a51a@mail.gmail.com>
> 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
next prev parent reply other threads:[~2010-03-04 7:39 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-04 2:37 Hui Zhu
2010-03-04 7:39 ` Joel Brobecker [this message]
2010-03-04 7:58 ` Hui Zhu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100304073855.GK2832@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=mark.kettenis@xs4all.nl \
--cc=teawater@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox