Rockbox.org home
release
dev builds
extras
themes manual
wiki
device status forums
mailing lists
IRC bugs
patches
dev guide



Rockbox mail archive

Subject: Suspicious code in playlist.c?

Suspicious code in playlist.c?

From: <postmaster_at_diffenbach.org>
Date: 2006-03-04

1. This code in playlist.c look suspicious: playlist->amount is only
tested against playlist->max_playlist_size AFTER playlist_>amount is
used as an array index. (Lines 510, 513, 515.)

2. And, if amount >= max_playlist_size, the function returns without
calling queue_post. Shouldn't we be able to use playlists up to and
including max_playlist_size? Instead we can only use playlists up to
max_playlist_size - 1.

3. Shouldn't we either be calling queue_post or reverting
playlist_info's state if we have to end because max_playlist_size has
been reached?

        
507 : if(*p != '#')
508 : {
509 : /* Store a new entry */
510 : hardeeps 1.89 playlist->indices[ playlist->amount ] = i+count;
511 : miipekk 1.133 #ifdef HAVE_DIRCACHE
512 : if (playlist->filenames)
513 : playlist->filenames[ playlist->amount ] = NULL;
514 : #endif
515 : hardeeps 1.89 if ( playlist->amount >=
playlist->max_playlist_size ) {
516 : hardeeps 1.76 display_buffer_full();
517 : return -1;
518 : }
519 : miipekk 1.133 playlist->amount++;
520 : hardeeps 1.76 }

3. Also, since playlist->amount is both read and written through a
pointer, this count be made slightly more efficient by using an
automatic variable. We can tighten it up even more if, as I suspect,
playlist->max_playlist_size does not change in the course of this
function. Of course, if either can be changed by other threads, this
shouldn't be done.
Received on Sat Mar 4 12:58:57 2006


Page was last modified "Jan 10 2012" The Rockbox Crew
aaa