From: <postmaster_at_diffenbach.org>
Date: Sat, 04 Mar 2006 06:58:21 -0500

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 2006-03-04