This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: acceptance rules (was: Re: [PATCH 3/6] x86/MPX: suppress base/index swapping ...)
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Jan Beulich <JBeulich at suse dot com>
- Cc: Alan Modra <amodra at gmail dot com>, Ian Taylor <iant at google dot com>, Nick Clifton <nickc at redhat dot com>, Richard Henderson <rth at redhat dot com>, kirill dot yukhin at intel dot com, Binutils <binutils at sourceware dot org>
- Date: Wed, 9 Oct 2013 09:45:35 -0700
- Subject: Re: acceptance rules (was: Re: [PATCH 3/6] x86/MPX: suppress base/index swapping ...)
- Authentication-results: sourceware.org; auth=none
- References: <5254349502000078000F9A3D at nat28 dot tlf dot novell dot com> <525435E002000078000F9A55 at nat28 dot tlf dot novell dot com> <CAMe9rOqDp1_mAdthvC=kJt_71K4b59kBXEAguGNMBKhrw24Upg at mail dot gmail dot com> <52543F7202000078000F9B07 at nat28 dot tlf dot novell dot com> <CAMe9rOptKg-2WiZ7PRjgmhUz=gWzexfy6BsDy_MPVP95c-uQvw at mail dot gmail dot com> <5254485102000078000F9B47 at nat28 dot tlf dot novell dot com> <CAMe9rOo1F66063acKDwdi207ducSf71jt0XxaZEm385RzJOTuw at mail dot gmail dot com> <52551EA202000078000F9D92 at nat28 dot tlf dot novell dot com>
On Wed, Oct 9, 2013 at 12:15 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 08.10.13 at 18:19, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>> I prefer a testcase together with the corresponding change,
>> instead of a jumbo testcase patch. I also don't agree every
>> MPX change you proposed. If it makes it easier to write
>> testcases, you can use a separate testcase file for each
>> change.
>
> Okay, so then I'll submit a monolithic patch combined with the
> testcase changes (once we sorted out eventual adjustments).
> Separate testcase files is not a desirable approach imo - what
> belongs together should stay together. As additional context:
> Getting the existing test case straightened took me significantly
> more time than fixing the actual bugs here, and I simply don't
You can open a bug report to report the issue
against the existing testcase.
> see myself wasting more time on this unless there's a _good_
> reason.
>
> And just to repeat - I'm very opposed to the idea of rejecting
> bug fixes just because of controversy about test cases. This
> isn't happening the first time (and is also not isolated to you as
> the x86 maintainer). I very much think that bug fixes ought to
> be acceptable in any case, and test cases ought to be optional.
> I can see this being more strict for enhancements, and even a
> requirement for new feature additions.
If a patch changes the assembler behavior, it should
be verified via a testcase to make sure that it does
what it is intended and stays that way.
> Yet in no case should - imo - badly written test cases be
> accepted just because this is better than no test case at all.
> But of course I realize that there's no guideline (or at least I'm
> unaware of there being any) on how a good test case would
> look like (my main requirements would be that they (a) don't
> test things to be valid that aren't and (b) use patterns instead
> of exact matches where precise values don't matter so that
> they can be extended without having to entirely replace them).
If a testcase, which is supposed to pass, contains invalid
instructions, we should just fix it. If you notice any issue
within binutils, including testcases, just open a bug report
against it. At least, there is a trail.
Thanks for your contribution.
--
H.J.
- References:
- [PATCH 0/6] x86: various MPX fixes
- [PATCH 3/6] x86/MPX: suppress base/index swapping in Intel mode for bndmk, bndldx, and bndstx
- Re: [PATCH 3/6] x86/MPX: suppress base/index swapping in Intel mode for bndmk, bndldx, and bndstx
- Re: [PATCH 3/6] x86/MPX: suppress base/index swapping in Intel mode for bndmk, bndldx, and bndstx
- Re: [PATCH 3/6] x86/MPX: suppress base/index swapping in Intel mode for bndmk, bndldx, and bndstx
- Re: [PATCH 3/6] x86/MPX: suppress base/index swapping in Intel mode for bndmk, bndldx, and bndstx
- Re: [PATCH 3/6] x86/MPX: suppress base/index swapping in Intel mode for bndmk, bndldx, and bndstx
- acceptance rules (was: Re: [PATCH 3/6] x86/MPX: suppress base/index swapping ...)