Re: [PATCH 3/4] tlbex.c: cleanup debug code

[Franck Bui-Huu <vagabon.xyz@gmail.com>]

Thiemo Seufer wrote:
> I don't like this patch. I wrote the code to
> a) print the handler before the (potentially fatal) memcpy. Touching
>    EBASE for the first time is a place where things like to go wrong.

Sorry I don't see what you mean...

Do you mean ebase (BTW this name sounds very local...) can be set to
an incorrect value and therefore the memcpy can get mad ?

If so I would say that with this patch applied you can easily guess
that an issue happen in this place because the log would be:

        Synthesized TLB refill handler (x instructions)
        <nothing>

and there would be no dump of the handler.

Whereas currently we would have:

        Synthesized TLB refill handler (x instructions)
        <handler dump>
        <nothing>

and now you don't know if the crash happen in the memcpy or later...

And it seems better to dump the code which is going to be _executed_
instead of a temporary buffer.

> b) avoid printing leading nops which are never executed. The trailing
>    nops have less potential for confusion.

Well, I disagree for 2 reasons:

   1/ By printing the leading nops you can detect a bug which would
      wrongly write in this unused part of the handler. That's one of
      the reason I added the handler instruction addresses BTW: to
      skip easily this part when reading the dump.

   2/ This is just debug code and it should not make the real code
      harder to read. Take a look at the end of
      build_r4000_tlb_refill_handler() function. It's not really
      obvious that this part is only for debug except the memcpy.

But you're the maintainer of this file so if you still think I should
drop this patch then I will.

Thanks.
                Franck