Dmitry, Yup,thank you point out that, need NULL pointer checking in calculate_video_dma_grab_packed() , I am not familiar with saa7146 too. wouldn't paper over it, just a fix to pass code review.
Thanks, Ethan On Sun, Jan 5, 2014 at 2:14 PM, Dmitry Torokhov <[email protected]> wrote: > On Sun, Jan 05, 2014 at 10:08:00AM +0800, Ethan Zhao wrote: >> Function saa7146_format_by_fourcc() may return NULL, reference of the >> returned >> result would cause NULL pointer issue without checking. >> >> Signed-off-by: Ethan Zhao <[email protected]> >> --- >> drivers/media/common/saa7146/saa7146_hlp.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/media/common/saa7146/saa7146_hlp.c >> b/drivers/media/common/saa7146/saa7146_hlp.c >> index be746d1..3e23eab 100644 >> --- a/drivers/media/common/saa7146/saa7146_hlp.c >> +++ b/drivers/media/common/saa7146/saa7146_hlp.c >> @@ -575,6 +575,7 @@ static void saa7146_set_position(struct saa7146_dev >> *dev, int w_x, int w_y, int >> */ >> u32 base = (u32)(unsigned long)vv->ov_fb.base; >> >> + int which = 1; >> struct saa7146_video_dma vdma1; >> >> /* calculate memory offsets for picture, look if we shall >> top-down-flip */ >> @@ -608,10 +609,14 @@ static void saa7146_set_position(struct saa7146_dev >> *dev, int w_x, int w_y, int >> vdma1.pitch *= -1; >> } >> >> - vdma1.base_page = sfmt->swap; >> + if (sfmt) >> + vdma1.base_page = sfmt->swap; >> + else >> + which = 0; >> + >> vdma1.num_line_byte = >> (vv->standard->v_field<<16)+vv->standard->h_pixels; >> >> - saa7146_write_out_dma(dev, 1, &vdma1); >> + saa7146_write_out_dma(dev, which, &vdma1); > > If saa7146_format_by_fourcc() returns NULL you'll crash much earlier > when trying to do > > int depth = sfmt->depth; > > I am not familiar with the code by it seems you are papering over the > problem. Will we ever get here with unknown pixel format? > > Thanks. > > -- > Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
