Rockbox mail archive
Subject: re: Bookmark Patch Comments
From: Benjamin (mailinglists_at_samuraipanda.com)
Thanks for the comments Daniel. Below are my responses.
> 1. don't memset() strings without reason all over
I didn't originally use memset, but I noticed that the code would
fail at various points for no apparent reason. Once I memset the
chars to 0, the problems vanished. I assumed this was because the
memset ensured there was no extraneous data left from the declaration.
Does it hurt to use memset?
> 2. the comment about the bookmark format is wrong, as your
> code uses "," as separator while the comment says
> "*". I also think you should make the separator a proper define.
For the life of me, I can't figure out how to edit the patch description
on sourceforge. It's been bugging me for a while that I can't update
it properly. Any help would be appreciated.
I had originally defined the separator, but Björn had requested it
not be defined for readability issues.
> Another more minor issue is that the code rules say "less
> than 80 columns" and you don't. In my editor it makes your
> long lines wrap weirdly.
I thought I had fixed them all. I'll look again and update any I
missed for my next build. Which lines are overlapped?
> Also, can you elaborate on what you mean with your
> "TBD" item 1 in the bookmark.c file?
TBD means "To Be Done" and was essentially a list of things I wanted
to done with the code by myself or others. The big issue is the
bookmark_play function (which I just copied the resume function and
removed some unnecessary code); there is probably a better more efficient
way of doing it.
Page was last modified "Jan 10 2012" The Rockbox Crew