Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#10478 closed Patch - Bug Fix (fixed)

Commercial flagging array initialization issue

Reported by: Gary Buhrmaster <gary.buhrmaster@…> Owned by: cpinkham
Priority: minor Milestone: 0.25
Component: MythTV - Mythcommflag Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no


clang detected in mythtv/programs/mythcommflag/ClassicCommDetector.cpp a warning of the form "warning: argument to 'sizeof' in 'memset' call is the same expression as the destination; did you mean to dereference it?" from the following code fragment:

memset(sc_histogram, 0, sizeof(sc_histogram));

My guess from reading the code is that the intent was to zero the entire array (and not just the size of the pointer) more or less as:

memset(sc_histogram, 0, sizeof(*sc_histogram)*(seconds+1));

(patch attached)

However, depending on what memory was allocated (and what its state was) this could substantially change (improve/worsen) commercial detection results. I presume that the SME for commercial flagging needs to carefully evaluate with their test cases the results of this proposed change.



Attachments (1)

ClassicCommDetector_cpp.diff (596 bytes) - added by Gary Buhrmaster <gary.buhrmaster@…> 9 years ago.

Download all attachments as: .zip

Change History (5)

Changed 9 years ago by Gary Buhrmaster <gary.buhrmaster@…>

comment:1 Changed 9 years ago by stuartm

Component: MythTV - GeneralMythTV - Mythcommflag
Milestone: unknown0.25
Owner: set to cpinkham
Version: UnspecifiedMaster Head

comment:2 Changed 9 years ago by Github

Resolution: fixed
Status: newclosed

Fix initialization of histograms

Based on a patch from Gary Buhrmaster <gary.buhrmaster@…>

Fixes #10478

Branch: master Changeset: 83759acfe5d01df3a8b1e718ba81559027ac1102

comment:3 Changed 9 years ago by Gary Buhrmaster <gary.buhrmaster@…>

The patch as modified still will only zero the first int of sc_histogram due to the (presumably) extra sizeof operand.

Did you perhaps mean to write:

memset(sc_histogram, 0, (seconds+1)*sizeof(int));

rather than:

memset(sc_histogram, 0, sizeof((seconds+1)*sizeof(int)));

comment:4 Changed 9 years ago by Github

Fix a stupid brain-o

Thanks again to Gary for finding this.

Refs #10478

Branch: master Changeset: 4c93463cabb5f26ab2ae773b355b3860895c1563

Note: See TracTickets for help on using tickets.