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

RESOLVED FIXED in Firefox 41

Status

()

Core
Widget: Gonk
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jseward, Assigned: jseward)

Tracking

Trunk
mozilla41
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
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.
(Assignee)

Comment 1

3 years ago
Created attachment 8609343 [details]
Valgrind complaints
(Assignee)

Comment 2

3 years ago
Created attachment 8610085 [details] [diff] [review]
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.
Attachment #8610085 - Flags: review?(mwu)

Comment 3

3 years ago
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.
Attachment #8610085 - Flags: review?(mwu) → review+
https://hg.mozilla.org/mozilla-central/rev/d744a0b3cd48
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.