[PATCH 3/8] d2d1/tests: Avoid out-of-bounds access when comparing segments

[Henri Verbeet]

On Tue, 11 Jan 2022 at 17:09, Stefan Brüns <stefan.bruens at rwth-aachen.de> wrote:
> On Dienstag, 11. Januar 2022 16:47:14 CET you wrote:
> > On Tue, 11 Jan 2022 at 08:55, Stefan Brüns <stefan.bruens at rwth-aachen.de>
> wrote:
> > > On Freitag, 7. Januar 2022 18:10:05 CET you wrote:
> > > > On Fri, 7 Jan 2022 at 09:00, Stefan Brüns <stefan.bruens at rwth-aachen.de>
> > >
> > > wrote:
> > > > > In case real and expected segment count differ, don't compare the
> > > > > segment type/position for extra segments.
> > > >
> > > > Unless it's e.g. a huge help when debugging failures, I'm not sure we
> > > > care; if the segment counts don't match the test is going to fail, one
> > > > way or another.
> > >
> > > This is an invalid memory access (though read-only). This should never be
> > > done, even in tests.
> >
> > That's not the position the Wine project has historically taken on
> > this kind of thing, at least when it comes to the tests. The basic
> > idea is that the test isn't supposed to fail, so when it does, we
> > don't particularly care about any follow-up damage. (Provided that's
> > limited to the test process.) I.e., think of ok() much like assert().
>
> The follow-up damage is not only to the process, but most importantly to the
> person trying to debug  the issue.
>
> In case the segments do not match (which is the more likely case, as it is
> just random data for the extraneous segments), it print totally bogus
> coordinates. This is confusing at best, printing a totally wrong message
> ("mismatching segments") and random coordinates.
>
> In case the segments do match (possible, as the allocated memory may reuse
> memory of an previous test, and coordinates of sequential tests are often
> similar), no coordinates are printed.
>
>
> For someone very familiar with the code, this might be expected. For anybody
> else debugging becomes harder. I have wasted half an hour getting what was
> actually going on, and this change would have saved me the time.
>
Right, that kind of thing was what I was referring to with "Unless
it's e.g. a huge help when debugging failures," earlier. In that case
I think this is fine.

Henri