public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@mips.com>
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: Simon Marchi <simon.marchi@ericsson.com>,
	<gdb-patches@sourceware.org>,	<binutils@sourceware.org>,
	Joel Brobecker <brobecker@adacore.com>,
	Fredrik Noring <noring@nocrew.org>
Subject: Re: [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'
Date: Thu, 21 Jun 2018 19:56:00 -0000	[thread overview]
Message-ID: <alpine.DEB.2.00.1806212029500.20622@tp.orcam.me.uk> (raw)
In-Reply-To: <5d5b6d47508d5f54ff2eb6e514a7da2c@polymtl.ca>

On Thu, 21 Jun 2018, Simon Marchi wrote:

> >  Thanks for the pointer.  Although it makes sense to me at first glance
> > that's quite a recent change to a long-established practice.  Perhaps it
> > could have been avoided by coding the regression analysis tools referred
> > more carefully, but I won't be questioning the decision at this point.
> 
> That's because of how DejaGNU formats test messages, for example when there is
> a timeout (as shown in the example on the wiki).  We don't have control over
> that, and we don't want "foo" and "foo (timeout)" to be considered as two
> different tests.

 I think you chose the wrong example, after all what's the problem for 
tools to notice that "foo (bar)" and "foo (bar) (timeout)" are the same 
test?  If the number of parenthesis pairs is different between two 
messages, then strip the extra ones along with the text between from the 
message that has more of them and only then compare the resulting strings.

 What the problem would be are different failures in different runs, e.g. 
if we have "foo (bar) (timeout)" in one and "foo (bar) (internal error)" 
in the other one, then indeed we have an issue.  We could strip 
parenthesis pairs one by one from both messages at a time, but then we'd 
incorrectly consider "foo (bar)" and "foo (baz)" the same test.

 Does it matter in reality?  I suppose so, given the outcome of the 
discussion referred from the wiki, although I don't see the scenario I 
have outlined here actually mentioned there (parts of the discussion seem 
to be missing though from the archive).

> That does not really apply to your case though.  I think here you can just
> remove the parenthesis, and maybe add a comma.
> 
>   mips_disassemble_test bar "move\tv0,a4" "disassemble ABI, n64"
> 
> As long as it's clear.

 OK, works for me.  I'll make that adjustment as I commit the change once 
the binutils parts have been approved.

 Thank you for your review.

  Maciej

  reply	other threads:[~2018-06-21 19:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-21  7:27 Maciej W. Rozycki
2018-06-21 17:52 ` Simon Marchi
2018-06-21 18:29   ` Maciej W. Rozycki
2018-06-21 19:19     ` Simon Marchi
2018-06-21 19:56       ` Maciej W. Rozycki [this message]
2018-06-21 20:06         ` Simon Marchi
2018-06-29 14:37         ` [PING][PATCH " Maciej W. Rozycki
2018-06-29 14:41           ` Maciej W. Rozycki
2018-07-02 15:57             ` Nick Clifton
2018-07-02 23:01               ` [committed v5] " Maciej W. Rozycki

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=alpine.DEB.2.00.1806212029500.20622@tp.orcam.me.uk \
    --to=macro@mips.com \
    --cc=binutils@sourceware.org \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=noring@nocrew.org \
    --cc=simon.marchi@ericsson.com \
    --cc=simon.marchi@polymtl.ca \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).