1167581 - Inconsistent pre-zeroing of ioctl buffers in gecko/hal/gonk/GonkFMRadio.cpp

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Gonk, defect)

Description

[Julian Seward [:jseward]]

GonkFMRadio.cpp has a number of ioctl calls of the form

  struct v4l2_somestruct result;
  int rc = ioctl(fd, VIDIOC_FOO, &result);
  if (rc >= 0) {
     ... use values in |result| ..
  }

so we are getting information from the kernel.  In most such cases
the structure is pre-zeroed before the ioctl, but this is not 
consistently done.  In particular there is a call

  int rc = ioctl(sRadioFD, VIDIOC_G_FREQUENCY, &freq);

done using a non-zeroed |freq| struct.  Per Hans Verkuil (cc'd) this is
not allowed; the struct must be zeroed before the call.

This also causes a lot of complaining from Valgrind.

Comment 1

[Julian Seward [:jseward]]

Attachment: Valgrind complaints

Comment 2

[Julian Seward [:jseward]]

Attachment: bug1167581-1.diff

Zero out three additional structs, for consistency with the rest of the
code in this file, and to make the calls comply with V4L2 requirements.

Comment 3

[Michael Wu [:mwu]]

Comment on attachment 8610085 [details] [diff] [review]
bug1167581-1.diff

Review of attachment 8610085 [details] [diff] [review]:
-----------------------------------------------------------------

::: hal/gonk/GonkFMRadio.cpp
@@ +310,4 @@
>    }
>  
>    struct v4l2_capability cap;
> +  memset(&cap, 0, sizeof(cap));

Will something like cap = {{0}} work here? Just for consistency with the way the rest of the code zeros structs.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d744a0b3cd48

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d744a0b3cd48