Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Bugs
  • Category ID3 / meta data
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.7.1
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by thomasjfox - 2011-02-16
Last edited by Buschel - 2011-02-17

FS#11948 - Issues reported by valgrind

Hello,

attached are four small issues identified by valgrind-3.6.0
using the rockbox SDL app build from r29321:

1.
==9141== Conditional jump or move depends on uninitialised value(s)
==9141== at 0×410221: get_mp3file_info (mp3data.c:467)
==9141== by 0×456555: getsonglength (mp3.c:67)
==9141== by 0x4568D9: get_mp3_metadata (mp3.c:176)
==9141== by 0x45429F: get_metadata (metadata.c:330)
==9141== by 0x4630DC: add_tagcache (tagcache.c:1852)
==9141== by 0x46715D: check_dir (tagcache.c:4366)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0x46732A: tagcache_build (tagcache.c:4446)
==9141== by 0x4674E5: tagcache_thread (tagcache.c:4582)

2.
==9141== Conditional jump or move depends on uninitialised value(s)
==9141== at 0x45999F: read_mp4_atom (mp4.c:140)
==9141== by 0x45A638: read_mp4_container (mp4.c:555)
==9141== by 0x45A7DA: read_mp4_container (mp4.c:593)
==9141== by 0x45A7DA: read_mp4_container (mp4.c:593)
==9141== by 0x45AC80: get_mp4_metadata (mp4.c:773)
==9141== by 0x45429F: get_metadata (metadata.c:330)
==9141== by 0x4630DC: add_tagcache (tagcache.c:1852)
==9141== by 0x46715D: check_dir (tagcache.c:4366)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0x46732A: tagcache_build (tagcache.c:4446)

3.
==9141== Conditional jump or move depends on uninitialised value(s)
==9141== at 0x4599C5: read_mp4_atom (mp4.c:150)
==9141== by 0x45A638: read_mp4_container (mp4.c:555)
==9141== by 0x45A7DA: read_mp4_container (mp4.c:593)
==9141== by 0x45A7DA: read_mp4_container (mp4.c:593)
==9141== by 0x45AC80: get_mp4_metadata (mp4.c:773)
==9141== by 0x45429F: get_metadata (metadata.c:330)
==9141== by 0x4630DC: add_tagcache (tagcache.c:1852)
==9141== by 0x46715D: check_dir (tagcache.c:4366)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0x46732A: tagcache_build (tagcache.c:4446)

4.
==9141== Conditional jump or move depends on uninitialised value(s)
==9141== at 0x45A644: read_mp4_container (mp4.c:562)
==9141== by 0x45A7DA: read_mp4_container (mp4.c:593)
==9141== by 0x45A7DA: read_mp4_container (mp4.c:593)
==9141== by 0x45AC80: get_mp4_metadata (mp4.c:773)
==9141== by 0x45429F: get_metadata (metadata.c:330)
==9141== by 0x4630DC: add_tagcache (tagcache.c:1852)
==9141== by 0x46715D: check_dir (tagcache.c:4366)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0×467121: check_dir (tagcache.c:4359)
==9141== by 0x46732A: tagcache_build (tagcache.c:4446)
==9141== by 0x4674E5: tagcache_thread (tagcache.c:4582)

Could someone with knowledge of the specific parsing code take a look
if this is valid or a false positive?

Wild guess: Files with corrupted metadata and we don’t do proper error checking
in some corner cases.

Thanks,
Thomas

Closed by  Buschel
2011-02-17 18:36
Reason for closing:  Fixed
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

Fixed with r29323.

Issues 2-4 could occur due to missing initialization of two variables (see mp4_v01.patch).

I am not sure about issue 1. To me it seems like “vbrheader[]” needs to be converted to “int” before left-shifting. Also “frame[]” is not initialized with zero. mp3data_v01.patch does both.

Can you please check again with both patches applied?

Good catch! With both patches applied, the issues are gone.
(rebuilt the database early this morning)

btw: I’ve found another uninitialized variables issue if you press rew/ff really fast.
Might be related to the rew/ff lockups mentioned on IRC.
I’ll open a separate ticket for that the next days.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing