Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Hui Zhu <teawater@gmail.com>
Cc: Michael Snyder <msnyder@vmware.com>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [RFA] i386-tdep.c, i386_process_record, document fall-through case.
Date: Tue, 08 Mar 2011 04:54:00 -0000	[thread overview]
Message-ID: <20110308043242.GI30306@adacore.com> (raw)
In-Reply-To: <AANLkTinNQXQWRvk1G8fiXGt1tJXwOGc=BzCDmv_fdf=r@mail.gmail.com>

> As my poor understanding of C language, break or not break are both OK
> for this part.

I'm going to be a little extremist, and I don't really mean what
I am about to ask, but: If the author of the code does not understand
the code, and no other maintainer is able to review associated patches,
is it time to remove that code?

Speaking about the patch itself, I had a look, and I think, from
what I understand, that, YES, the fallthrough is intended. IMO,
it would have been clearer to write the code as follow:

    case 0xc4:      /* les Gv */
    case 0xc5:      /* lds Gv */
    case 0x0fb2:    /* lss Gv */
    case 0x0fb4:    /* lfs Gv */
    case 0x0fb5:    /* lgs Gv */
      if ((opcode == 0xc4 || opcode == 0xc5)
          && ir.regmap[X86_RECORD_R8_REGNUM])
        {
          ir.addr -= 1;
          goto no_support;
        }
      if (i386_record_modrm (&ir))
        return -1;

(thus, not requiring a fallthrough)

So patch is approved.

-- 
Joel


  reply	other threads:[~2011-03-08  4:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-02 22:34 Michael Snyder
2011-03-02 22:47 ` Mark Kettenis
2011-03-04 19:05 ` Michael Snyder
2011-03-07  9:01   ` Hui Zhu
2011-03-07 19:06     ` Michael Snyder
2011-03-08  4:35       ` Hui Zhu
2011-03-08  4:54         ` Joel Brobecker [this message]
2011-03-08  4:58           ` Hui Zhu
2011-03-08 18:50             ` Michael Snyder
2011-03-08 17:20           ` Tom Tromey

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=20110308043242.GI30306@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=msnyder@vmware.com \
    --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