All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] kms_atomic: drop unnecessary connector looping from plane_primary test
@ 2016-02-26 16:58 Matt Roper
  2016-02-26 17:00 ` Daniel Stone
  0 siblings, 1 reply; 3+ messages in thread
From: Matt Roper @ 2016-02-26 16:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Stone

Local variable num_connectors is never initialized before being
auto-incremented in the loop.  If we wind up with a non-zero garbage
value, it will lead us to try to write to an out-of-bounds array index.
We should probably initialize it to zero before use.

However on closer inspection, the plane_primary test doesn't actually
wind up using the connector list or number of connectors, so just remove
the whole block of code; it was probably brought in by accident as part
of a copy-paste operation.

Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 tests/kms_atomic.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
index c8b8b78..2f3080a 100644
--- a/tests/kms_atomic.c
+++ b/tests/kms_atomic.c
@@ -909,19 +909,7 @@ static void plane_primary(struct kms_atomic_crtc_state *crtc,
 	struct kms_atomic_plane_state plane = *plane_old;
 	uint32_t format = plane_get_igt_format(&plane);
 	drmModeAtomicReq *req = drmModeAtomicAlloc();
-	uint32_t *connectors;
-	int num_connectors;
 	struct igt_fb fb;
-	int i;
-
-	connectors = calloc(crtc->state->num_connectors, sizeof(*connectors));
-	igt_assert(connectors);
-
-	for (i = 0; i < crtc->state->num_connectors; i++) {
-		if (crtc->state->connectors[i].crtc_id == crtc->obj)
-			connectors[num_connectors++] =
-				crtc->state->connectors[i].obj;
-	}
 
 	igt_require(format != 0);
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH i-g-t] kms_atomic: drop unnecessary connector looping from plane_primary test
  2016-02-26 16:58 [PATCH i-g-t] kms_atomic: drop unnecessary connector looping from plane_primary test Matt Roper
@ 2016-02-26 17:00 ` Daniel Stone
  2016-02-26 17:21   ` Matt Roper
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Stone @ 2016-02-26 17:00 UTC (permalink / raw)
  To: Matt Roper, intel-gfx

Hi,

On Fri, 2016-02-26 at 08:58 -0800, Matt Roper wrote:
> Local variable num_connectors is never initialized before being
> auto-incremented in the loop.  If we wind up with a non-zero garbage
> value, it will lead us to try to write to an out-of-bounds array
> index.
> We should probably initialize it to zero before use.
> 
> However on closer inspection, the plane_primary test doesn't actually
> wind up using the connector list or number of connectors, so just
> remove
> the whole block of code; it was probably brought in by accident as
> part
> of a copy-paste operation.

History rather than copy & paste; originally crtc_commit_*() was
inlined. But the rest of your analysis is totally correct; thanks for
finding this!

Reviewed-by: Daniel Stone <daniels@collabora.com>

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH i-g-t] kms_atomic: drop unnecessary connector looping from plane_primary test
  2016-02-26 17:00 ` Daniel Stone
@ 2016-02-26 17:21   ` Matt Roper
  0 siblings, 0 replies; 3+ messages in thread
From: Matt Roper @ 2016-02-26 17:21 UTC (permalink / raw)
  To: Daniel Stone; +Cc: intel-gfx

On Fri, Feb 26, 2016 at 05:00:21PM +0000, Daniel Stone wrote:
> Hi,
> 
> On Fri, 2016-02-26 at 08:58 -0800, Matt Roper wrote:
> > Local variable num_connectors is never initialized before being
> > auto-incremented in the loop.  If we wind up with a non-zero garbage
> > value, it will lead us to try to write to an out-of-bounds array
> > index.
> > We should probably initialize it to zero before use.
> > 
> > However on closer inspection, the plane_primary test doesn't actually
> > wind up using the connector list or number of connectors, so just
> > remove
> > the whole block of code; it was probably brought in by accident as
> > part
> > of a copy-paste operation.
> 
> History rather than copy & paste; originally crtc_commit_*() was
> inlined. But the rest of your analysis is totally correct; thanks for
> finding this!
> 
> Reviewed-by: Daniel Stone <daniels@collabora.com>

Thanks for the speedy review!  Pushed to fdo.


Matt

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-02-26 17:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-26 16:58 [PATCH i-g-t] kms_atomic: drop unnecessary connector looping from plane_primary test Matt Roper
2016-02-26 17:00 ` Daniel Stone
2016-02-26 17:21   ` Matt Roper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.