Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#5567 closed defect (fixed)

mythmusic defects

Reported by: Erik Hovland <erik@…> Owned by: paulh
Priority: minor Milestone: 0.22
Component: mythmusic Version: head
Severity: low Keywords:
Cc: Ticket locked: no

Description

There are various defects in mythmusic. The attached patch attempts to address them all. There is a header to the patch to describe each defect.

Attachments (1)

mythmusic-defects.patch (15.0 KB) - added by Erik Hovland <erik@…> 11 years ago.
Attempts to fix defects in mythmusic

Download all attachments as: .zip

Change History (4)

Changed 11 years ago by Erik Hovland <erik@…>

Attachment: mythmusic-defects.patch added

Attempts to fix defects in mythmusic

comment:1 Changed 11 years ago by paulh

Milestone: unknown0.22
Owner: changed from Isaac Richards to paulh
Status: newassigned

Erik I assume reverting Daniel's changes in encoder.cpp was a mistake?

The only other change I've made was we might as well use Qt's QDir::home() to get the home directory rather than mess about with environment variables.

comment:2 Changed 11 years ago by paulh

Resolution: fixed
Status: assignedclosed

(In [18198]) Various MythMusic defect fixes from Erik Hovland. Fixes #5567.

  1. By moving the 'return 0;' inside the cpp clause an unreachable defect is not triggered.
  2. If a parent isn't found then ripTrack could be leaked.
  3. item declaration hides the item parameter in the member function.
  4. Declaration of outfile hides parameter in ctor.
  5. Since the else clause guarantees that the function will end the second 'return -1;' is not necessary. If the else clause is just moved to the end and the extra return removed it has the same effect.
  6. Using sprintf with getenv is dangerous and could cause the string to be overflowed. Use Qt's QDir::home() function to get the home directory instead.
  7. m_decoder is the wrong pointer to check. The one to check is the one given by the dyn cast.
  8. else return missing from conditional clause. Without it there then node could be dereferenced even though it is invalid (and a segfault could happen).
  9. tmpdata might be null and then dereferenced when getting album and artist.
  10. Use of tmpnam is dangerous. Using createTempFile from libmyth.
  11. PLField might be null. Dereferencing it could end in segfault.
  12. Assignment of item in do/while loop is unnecessary.
  13. Removal of dead code.

comment:3 Changed 11 years ago by anonymous

Erik I assume reverting Daniel's changes in encoder.cpp was a mistake?

Yes, once Daniel's change was reverted the other nonesense I put in was no longer relavent.

Thanks for looking into this one.

Note: See TracTickets for help on using tickets.