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

[Hui Zhu <teawater at gmail dot 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
>