[media-m5602-po1030] question about return value check

[Gustavo A. R. Silva]

Hello everybody,

While looking into Coverity ID 1227044 I ran into the following piece  
of code at drivers/media/usb/gspca/m5602/m5602_po1030.c:280:

280int po1030_start(struct sd *sd)
281{
282        struct cam *cam = &sd->gspca_dev.cam;
283        int i, err = 0;
284        int width = cam->cam_mode[sd->gspca_dev.curr_mode].width;
285        int height = cam->cam_mode[sd->gspca_dev.curr_mode].height;
286        int ver_offs = cam->cam_mode[sd->gspca_dev.curr_mode].priv;
287        u8 data;
288
289        switch (width) {
290        case 320:
291                data = PO1030_SUBSAMPLING;
292                err = m5602_write_sensor(sd, PO1030_CONTROL3, &data, 1);
293                if (err < 0)
294                        return err;
295
296                data = ((width + 3) >> 8) & 0xff;
297                err = m5602_write_sensor(sd, PO1030_WINDOWWIDTH_H,  
&data, 1);
298                if (err < 0)
299                        return err;
300
301                data = (width + 3) & 0xff;
302                err = m5602_write_sensor(sd, PO1030_WINDOWWIDTH_L,  
&data, 1);
303                if (err < 0)
304                        return err;
305
306                data = ((height + 1) >> 8) & 0xff;
307                err = m5602_write_sensor(sd, PO1030_WINDOWHEIGHT_H,  
&data, 1);
308                if (err < 0)
309                        return err;
310
311                data = (height + 1) & 0xff;
312                err = m5602_write_sensor(sd, PO1030_WINDOWHEIGHT_L,  
&data, 1);
313
314                height += 6;
315                width -= 1;
316                break;
317
318        case 640:
319                data = 0;
320                err = m5602_write_sensor(sd, PO1030_CONTROL3, &data, 1);
321                if (err < 0)
322                        return err;
323
324                data = ((width + 7) >> 8) & 0xff;
325                err = m5602_write_sensor(sd, PO1030_WINDOWWIDTH_H,  
&data, 1);
326                if (err < 0)
327                        return err;
328
329                data = (width + 7) & 0xff;
330                err = m5602_write_sensor(sd, PO1030_WINDOWWIDTH_L,  
&data, 1);
331                if (err < 0)
332                        return err;
333
334                data = ((height + 3) >> 8) & 0xff;
335                err = m5602_write_sensor(sd, PO1030_WINDOWHEIGHT_H,  
&data, 1);
336                if (err < 0)
337                        return err;
338
339                data = (height + 3) & 0xff;
340                err = m5602_write_sensor(sd, PO1030_WINDOWHEIGHT_L,  
&data, 1);
341
342                height += 12;
343                width -= 2;
344                break;
345        }
346        err = m5602_write_bridge(sd, M5602_XB_SENSOR_TYPE, 0x0c);
347        if (err < 0)
348                return err;
349
350        err = m5602_write_bridge(sd, M5602_XB_LINE_OF_FRAME_H, 0x81);
351        if (err < 0)
352                return err;
353
354        err = m5602_write_bridge(sd, M5602_XB_PIX_OF_LINE_H, 0x82);
355        if (err < 0)
356                return err;
357
358        err = m5602_write_bridge(sd, M5602_XB_SIG_INI, 0x01);
359        if (err < 0)
360                return err;
361
362        err = m5602_write_bridge(sd, M5602_XB_VSYNC_PARA,
363                                 ((ver_offs >> 8) & 0xff));
364        if (err < 0)
365                return err;
366
367        err = m5602_write_bridge(sd, M5602_XB_VSYNC_PARA, (ver_offs  
& 0xff));
368        if (err < 0)
369                return err;
370
371        for (i = 0; i < 2 && !err; i++)
372                err = m5602_write_bridge(sd, M5602_XB_VSYNC_PARA, 0);
373        if (err < 0)
374                return err;
375
376        err = m5602_write_bridge(sd, M5602_XB_VSYNC_PARA, (height  
>> 8) & 0xff);
377        if (err < 0)
378                return err;
379
380        err = m5602_write_bridge(sd, M5602_XB_VSYNC_PARA, (height & 0xff));
381        if (err < 0)
382                return err;
383
384        for (i = 0; i < 2 && !err; i++)
385                err = m5602_write_bridge(sd, M5602_XB_VSYNC_PARA, 0);
386
387        for (i = 0; i < 2 && !err; i++)
388                err = m5602_write_bridge(sd, M5602_XB_SIG_INI, 0);
389
390        for (i = 0; i < 2 && !err; i++)
391                err = m5602_write_bridge(sd, M5602_XB_HSYNC_PARA, 0);
392        if (err < 0)
393                return err;
394
395        err = m5602_write_bridge(sd, M5602_XB_HSYNC_PARA, (width >>  
8) & 0xff);
396        if (err < 0)
397                return err;
398
399        err = m5602_write_bridge(sd, M5602_XB_HSYNC_PARA, (width & 0xff));
400        if (err < 0)
401                return err;
402
403        err = m5602_write_bridge(sd, M5602_XB_SIG_INI, 0);
404        return err;
405}

The issue here is that the value contained in variable _err_ at lines  
312 and 340 is not being evaluated as it happens in other instances  
after calling m5602_write_sensor().

I'm suspicious this is not the original intention and maybe a patch  
like the following could be applied:

--- a/drivers/media/usb/gspca/m5602/m5602_po1030.c+++  
b/drivers/media/usb/gspca/m5602/m5602_po1030.c@@ -310,6 +310,8 @@ int  
po1030_start(struct sd *sd)

                data = (height + 1) & 0xff;
                err = m5602_write_sensor(sd, PO1030_WINDOWHEIGHT_L, &data, 1);
+               if (err < 0)
+                       return err;

                height += 6;
                width -= 1;
@@ -338,6 +340,8 @@ int po1030_start(struct sd *sd)

                data = (height + 3) & 0xff;
                err = m5602_write_sensor(sd, PO1030_WINDOWHEIGHT_L, &data, 1);
+               if (err < 0)
+                       return err;

                height += 12;
                width -= 2;


What do you think?

It would be great to hear your opinions about it.

I'd really appreciate any comment on this.

Thank you!
--
Gustavo A. R. Silva