qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Laurent Desnogues" <laurent.desnogues@gmail.com>
To: Paul Brook <paul@codesourcery.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] Re: [PATCH][ARM] Fix wrong destination register for smuad, smusd, smlad, smlsd
Date: Wed, 25 Jun 2008 19:15:45 +0200	[thread overview]
Message-ID: <761ea48b0806251015t4088f674hcacbc256b7a6dc0f@mail.gmail.com> (raw)
In-Reply-To: <200806251758.58313.paul@codesourcery.com>

On Wed, Jun 25, 2008 at 6:58 PM, Paul Brook <paul@codesourcery.com> wrote:
> On Wednesday 25 June 2008, Laurent Desnogues wrote:
>> 2008/6/23 Laurent Desnogues <laurent.desnogues@gmail.com>:
>> > smuad, smusd, smlad, smlsd write the wrong register, resulting in
>> > PC corruption.
>>
>> Here is a better patch.
>>
>> I will stop posting patches as I have the feeling the maintainer
>> doesn't care.  He might be rewriting everything and so my
>> patches are useless and a waste of time for everyone :-)
>
> TBH It's hard to tell if these are well tested patches, or just quick hacks
> that you're throwing over the wall.
>
> As above where it took you two tries. This isn't bad per se, but compete lack
> of explanation about what's different doesn't help. Some of your other
> patches have been prefixed with "I did not check the correctness of that
> instruction in general, I only made a change that looked logical" and "These
> are all *wild guesses*". As we know from the recent Debian SSL debacle
> making "a change that looked logical" can be fairly disastrous :-)

I am certainly over cautious as everyone should be when he doesn't
master all of a software :-)  So indeed when I propose some "wild
guess" it's all related to parts of qemu I am not sure to understand
and generally doesn't come with a patch. On the other hand when I
post a fix for the behaviour of an instruction I am sure it's OK except
when it contains multiple bugs (as was the case for that instruction).
I am now doing explicit tests to check each instruction does what
it should, so this mistake should not happen anymore.

> This means I'm unwilling to accept the patches a face value, and need to go
> through them with a fine tooth comb. This takes time, and you go in the queue
> with the dozens of other of patches, bugs and new features that need my
> attention.

That's enough of a feedback to me, I am not expecting you to
commit anything within a minute or even within days.  The lack of
any feedback from you made me wonder if you were not within a
complete rewrite of the ARM target that would have made my
patches useless and just some noise on the list :)

Anyway, are you interested in a patch that corrects enough of
v6 instructions that FFmpeg for ARMv6 works?  Would FFmpeg
be considered as a good enough test?


Laurent

  reply	other threads:[~2008-06-25 17:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-23  9:40 [Qemu-devel] [PATCH][ARM] Fix wrong destination register for smuad, smusd, smlad, smlsd Laurent Desnogues
2008-06-25 15:25 ` [Qemu-devel] " Laurent Desnogues
2008-06-25 16:58   ` Paul Brook
2008-06-25 17:15     ` Laurent Desnogues [this message]
2008-06-25 17:27       ` Paul Brook
2008-06-25 19:00         ` Laurent Desnogues
2008-06-25 19:11         ` Vincent Palatin

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=761ea48b0806251015t4088f674hcacbc256b7a6dc0f@mail.gmail.com \
    --to=laurent.desnogues@gmail.com \
    --cc=paul@codesourcery.com \
    --cc=qemu-devel@nongnu.org \
    /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;
as well as URLs for NNTP newsgroup(s).