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.


Reply via email to