From: Stuart Tedford <stuart.tedford_at_piresearch.co.uk>
Date: Wed, 24 Jul 2002 08:45:32 +0100

I have finally had some time to look at the scroller code and found the bug
- it's a good one and worth explaining.

The scrollinfo struct uses a char to index the text array. When that char
(being of the signed variety) reaches 128, it actually wraps around to -128
and then indexes (via a cast to an int) 128 bytes before the beginning of
the array in memory. This explains the behaviour seen where it would
display 128 bytes of rubbish after reaching 128, and then recover when it
finally came back to the beginning of the array.

The fix is simple, but I am unfamiliar with the patch generation method
(maybe someone could post some instructions), so I will write out the change
(it's only one line) and could someone commit it to cvs for me please.

in "Rockbox\FIRMWARE\DRIVERS\lcd.c" line 108
change "char offset;"
to "int offset;"

This is in the scrollinfo struct. While you are at it, you may as well
change the array member above it "char text[257];" to "char text[MAX_PATH];"
since it is mostly used to display filenames.

Stu.


> -----Original Message-----
> From: Stuart Tedford [mailto:stuart.tedford_at_piresearch.co.uk]
> Sent: 10 July 2002 18:39
> To: 'rockbox_at_cool.haxx.se'
> Subject: RE: Possible Filename Buffer Overrun
>
>
> > I noticed that TREE_MAX_FILENAMELEN is defined to be 128 in
> > tree.c. The
> > actual filename looks like it can come from tree.c and playlist.c.
>
> Yeah, I noticed that as well. peek_next_track() seems to be where the
> filename comes from.
>
> I also noticed that it takes 25 seconds for the scroller to
> scroll 128 bytes
> (with default timing) and once the first readable 128 bytes
> of the filename
> have been displayed, the garbage is displayed for - you
> guessed it - 25
> seconds before recovering and starting all over again.
>
> Lots of the code that deals with these buffers is explicitly
> putting a NULL
> at the end of the buffer (almost always 256 bytes - except in the case
> above) so when the scroller reaches that NULL, it starts
> again - I like
> defensive programming.
>
> The problem seems to be related to this 128 byte buffer, but
> it needs more
> investigation. I'm concentrating on getting the build
> environment working
> before I can really do anything.
>
> Stu.
>
Received on 2002-07-24