atmel-isc: add the isc pipeline function

Hello Songjun Wu,

The patch 93d4a26c3dab: "[media] atmel-isc: add the isc pipeline function" from Jan 24, 2017, leads to the following static checker warning:

drivers/media/platform/atmel/atmel-isc.c:1494 isc_formats_init()
error: we previously assumed 'fmt' could be null (see line 1480)

drivers/media/platform/atmel/atmel-isc.c
1459 static int isc_formats_init(struct isc_device *isc) 1460 {

  1461          struct isc_format *fmt;
  1462          struct v4l2_subdev *subdev = isc->current_subdev->sd;
  1463          unsigned int num_fmts, i, j;
  1464          struct v4l2_subdev_mbus_code_enum mbus_code = {
  1465                  .which = V4L2_SUBDEV_FORMAT_ACTIVE,
  1466          };
  1467  
  1468          fmt = &isc_formats[0];
  1469          for (i = 0; i < ARRAY_SIZE(isc_formats); i++) {
  1470                  fmt->isc_support = false;
  1471                  fmt->sd_support = false;
  1472  
  1473                  fmt++;
  1474          }
  1475  
  1476          while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
  1477                 NULL, &mbus_code)) {
  1478                  mbus_code.index++;
  1479                  fmt = find_format_by_code(mbus_code.code, &i);
  1480                  if (!fmt)
                             ^^^

Check for NULL.

  1481                          continue;
  1482  
  1483                  fmt->sd_support = true;
  1484  
  1485                  if (i <= RAW_FMT_IND_END) {
  1486                          for (j = ISC_FMT_IND_START; j <= ISC_FMT_IND_END; j++)
  1487                                  isc_formats[j].isc_support = true;
  1488  
  1489                          isc->raw_fmt = fmt;
  1490                  }
  1491          }
  1492  

We probably want to set: fmt = &isc_formats[0]; before the start of this loop.

1493 for (i = 0, num_fmts = 0; i < ARRAY_SIZE(isc_formats); i++) {
1494 if (fmt->isc_support || fmt->sd_support)

                            ^^^^^^^^^^^^^^^^

Unchecked dereference.

1495 num_fmts++;
1496
1497 fmt++;
1498 }
1499
1500 if (!num_fmts)
1501 return -ENXIO;

regards,
dan carpenter

Hi Dan,

Thank you very much for your bug report.
Then I have question about 'error: we previously assumed 'fmt' could be null (see line 1480)' Do you mean that the code should be written like 'if (fmt == NULL)'?

No. Imagine the v4l2_subdev_call() loop exits with "fmt" set to NULL. It will cause a crash.

Please re-read my original email because I think you may have meant to reset fmt after that loop.

regards,
dan carpenter

Hi Dan,

I understand now, thank you very much.