OMAPDSS: reading past end of array in dispc_dump_regs()

Started by Dan Carpenter on Dec 14, 2012 · 3 messages from 2 authors · Last reply Dec 17, 2012

Subsystems (2)

Maintainers (2)

STALE5079d

open the first message on its own page

Message details and related content
We added another kind of plane in 66a0f9e4ac "OMAPDSS: Use WB fifo for
GFX overlay" so this array needs a new entry as well.

Signed-off-by: Dan Carpenter <redacted>
---
Static checker work.  I don't have a way to test this.
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index fedbd2c..bfe62cc 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3163,6 +3163,7 @@ static void dispc_dump_regs(struct seq_file *s)
 		[OMAP_DSS_VIDEO1]	= "VID1",
 		[OMAP_DSS_VIDEO2]	= "VID2",
 		[OMAP_DSS_VIDEO3]	= "VID3",
+		[OMAP_DSS_WB]		= "WB",
 	};
 	const char **p_names;
 
Message details and related content
On 2012-12-14 17:01, Dan Carpenter wrote:
quoted hunk
We added another kind of plane in 66a0f9e4ac "OMAPDSS: Use WB fifo for
GFX overlay" so this array needs a new entry as well.

Signed-off-by: Dan Carpenter <redacted>
---
Static checker work.  I don't have a way to test this.
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index fedbd2c..bfe62cc 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3163,6 +3163,7 @@ static void dispc_dump_regs(struct seq_file *s)
 		[OMAP_DSS_VIDEO1]	= "VID1",
 		[OMAP_DSS_VIDEO2]	= "VID2",
 		[OMAP_DSS_VIDEO3]	= "VID3",
+		[OMAP_DSS_WB]		= "WB",
 	};
 	const char **p_names;
 
We don't count WB as an overlay currently, as it's handled a bit
differently, so we never try to access that array with OMAP_DSS_WB. We
don't actually dump any WB related registers currently, it seems.

So I think I'll leave this out for now.

Why does the static checker think OMAP_DSS_WB is needed in the array? I
wonder if I'm reading the code wrong, and we indeed do access the array
with OMAP_DSS_WB...

 Tomi

Message details and related content
On Mon, Dec 17, 2012 at 02:09:00PM +0200, Tomi Valkeinen wrote:
Why does the static checker think OMAP_DSS_WB is needed in the array?
drivers/video/omap2/dss/dispc.c +3284

  3274  #define DISPC_REG(plane, name, i) name(plane, i)
  3275  #define DUMPREG(plane, name, i) \
  3276          seq_printf(s, "%s_%d(%s)%*s %08x\n", #name, i, p_names[plane], \
  3277          (int)(46 - strlen(#name) - strlen(p_names[plane])), " ", \
  3278          dispc_read_reg(DISPC_REG(plane, name, i)))
  3279  
  3280          /* Video pipeline coefficient registers */
  3281  
  3282          /* start from OMAP_DSS_VIDEO1 */
  3283          for (i = 1; i < dss_feat_get_num_ovls(); i++) {
  3284                  for (j = 0; j < 8; j++)
  3285                          DUMPREG(i, DISPC_OVL_FIR_COEF_H, j);

The logic here is that we pass i to DISPC_OVL_FIR_COEF_H() which
passes i to DISPC_FIR_COEF_H_OFFSET().  Anything higher than
OMAP_DSS_WB will trigger a BUG() in DISPC_FIR_COEF_H_OFFSET().

So it's not rock hard logic at all.

regards,
dan carpenter
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help