Re: [RFA] i386-tdep.c, i386_process_record, document fall-through case.

[Hui Zhu <teawater@gmail.com>]

And about the code:
    case 0xc4:    /* les Gv */
    case 0xc5:    /* lds Gv */
      if (ir.regmap[X86_RECORD_R8_REGNUM])
        {
	  ir.addr -= 1;
	  goto no_support;
	}

      switch (opcode)
	{
	case 0xc4:    /* les Gv */
	  regnum = X86_RECORD_ES_REGNUM;
	  break;
	case 0xc5:    /* lds Gv */
	  regnum = X86_RECORD_DS_REGNUM;
	  break;

I check my code didn't very clear.  This part is a "/* ELSE FALL THROUGH */".


On Tue, Mar 8, 2011 at 12:32, Joel Brobecker <brobecker@adacore.com> wrote:
>> 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?

Interesting.

Please go ahead if you want.  :)

BTW If somebody say something wrong about his code, his code need
prepare to be removed, right?
I suggest you post your words to the website of gdb.  That will be
powerful motto for gdb club.  :)

Thanks,
Hui

>
> 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
>