This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]