diff options
author | William Wilgus <wilgus.william@gmail.com> | 2022-12-08 17:49:52 -0500 |
---|---|---|
committer | William Wilgus <me.theuser@yahoo.com> | 2022-12-08 17:55:36 -0500 |
commit | 7a00ad72e485b5994aff9d09b1e5213d0a2f0455 (patch) | |
tree | d7e5b0c2e6e6550b87ce9f057d3895c497c28bda /apps/bookmark.c | |
parent | 3555e84a7a0ce769305cbd19fc145619c5f7615b (diff) | |
download | rockbox-7a00ad72e485b5994aff9d09b1e5213d0a2f0455.tar.gz rockbox-7a00ad72e485b5994aff9d09b1e5213d0a2f0455.zip |
[Bug Fix] bookmark.c failure to compare existing tracks and playlist
the underlying global buffer fo bookmark is overwritten by the read function so
instead make a hash of the playlist and track names
might needa better hash but hopefully CRC32 is sufficient
Change-Id: Ie25dbb62e664b804e4f858d9e0cc248fdc8c9895
Diffstat (limited to 'apps/bookmark.c')
-rw-r--r-- | apps/bookmark.c | 71 |
1 files changed, 43 insertions, 28 deletions
diff --git a/apps/bookmark.c b/apps/bookmark.c index 48afc8d36e..68c10b36e7 100644 --- a/apps/bookmark.c +++ b/apps/bookmark.c | |||
@@ -86,6 +86,11 @@ struct resume_info{ | |||
86 | #define TEMP_BUF_SIZE (MAX(MAX_BOOKMARK_SIZE, MAX_PATH + 1)) | 86 | #define TEMP_BUF_SIZE (MAX(MAX_BOOKMARK_SIZE, MAX_PATH + 1)) |
87 | static char global_temp_buffer[TEMP_BUF_SIZE]; | 87 | static char global_temp_buffer[TEMP_BUF_SIZE]; |
88 | 88 | ||
89 | static inline void get_hash(const char *key, uint32_t *hash, int len) | ||
90 | { | ||
91 | *hash = crc_32(key, len, *hash); /* this is probably sufficient */ | ||
92 | } | ||
93 | |||
89 | static const char* skip_tokens(const char* s, int ntokens) | 94 | static const char* skip_tokens(const char* s, int ntokens) |
90 | { | 95 | { |
91 | for (int i = 0; i < ntokens; i++) | 96 | for (int i = 0; i < ntokens; i++) |
@@ -120,17 +125,33 @@ static long long_token(const char **s) | |||
120 | /* Get the name of the playlist and the name of the track from a bookmark. */ | 125 | /* Get the name of the playlist and the name of the track from a bookmark. */ |
121 | /* Returns true iff both were extracted. */ | 126 | /* Returns true iff both were extracted. */ |
122 | /*-------------------------------------------------------------------------*/ | 127 | /*-------------------------------------------------------------------------*/ |
123 | static bool bookmark_get_playlist_and_track(const char *bookmark, | 128 | static bool bookmark_get_playlist_and_track_hash(const char *bookmark, |
124 | const char **pl_start, | 129 | uint32_t *pl_hash, |
125 | const char **pl_end, | 130 | uint32_t *track_hash) |
126 | const char **track) | ||
127 | { | 131 | { |
132 | *pl_hash = 0; | ||
133 | *track_hash = 0; | ||
134 | int pl_len; | ||
135 | const char *pl_start, *pl_end, *track; | ||
136 | |||
128 | logf("%s", __func__); | 137 | logf("%s", __func__); |
129 | *pl_start = strchr(bookmark,'/'); | 138 | |
130 | if (!(*pl_start)) | 139 | pl_start = strchr(bookmark,'/'); |
140 | if (!(pl_start)) | ||
131 | return false; | 141 | return false; |
132 | *pl_end = skip_tokens(*pl_start, 1) - 1; | 142 | |
133 | *track = *pl_end + 1; | 143 | pl_end = skip_tokens(pl_start, 1) - 1; |
144 | pl_len = pl_end - pl_start; | ||
145 | |||
146 | track = pl_end + 1; | ||
147 | get_hash(pl_start, pl_hash, pl_len); | ||
148 | |||
149 | if (global_settings.usemrb == BOOKMARK_ONE_PER_TRACK) | ||
150 | { | ||
151 | get_hash(track, track_hash, strlen(track)); | ||
152 | } | ||
153 | |||
154 | |||
134 | return true; | 155 | return true; |
135 | } | 156 | } |
136 | 157 | ||
@@ -262,13 +283,11 @@ static bool add_bookmark(const char* bookmark_file_name, | |||
262 | int temp_bookmark_file = 0; | 283 | int temp_bookmark_file = 0; |
263 | int bookmark_file = 0; | 284 | int bookmark_file = 0; |
264 | int bookmark_count = 0; | 285 | int bookmark_count = 0; |
265 | int pl_len = 0, bm_pl_len; | ||
266 | const char *pl_start = NULL, *bm_pl_start; | ||
267 | const char *pl_end = NULL, *bm_pl_end; | ||
268 | const char *track = NULL, *bm_track; | ||
269 | bool comp_playlist = false; | 286 | bool comp_playlist = false; |
270 | bool comp_track = false; | 287 | bool comp_track = false; |
271 | bool equal; | 288 | bool equal; |
289 | uint32_t pl_hash, pl_track_hash; | ||
290 | uint32_t bm_pl_hash, bm_pl_track_hash; | ||
272 | 291 | ||
273 | /* Opening up a temp bookmark file */ | 292 | /* Opening up a temp bookmark file */ |
274 | temp_bookmark_file = open_temp_bookmark(fnamebuf, | 293 | temp_bookmark_file = open_temp_bookmark(fnamebuf, |
@@ -282,20 +301,22 @@ static bool add_bookmark(const char* bookmark_file_name, | |||
282 | if (most_recent && ((global_settings.usemrb == BOOKMARK_ONE_PER_PLAYLIST) | 301 | if (most_recent && ((global_settings.usemrb == BOOKMARK_ONE_PER_PLAYLIST) |
283 | || (global_settings.usemrb == BOOKMARK_ONE_PER_TRACK))) | 302 | || (global_settings.usemrb == BOOKMARK_ONE_PER_TRACK))) |
284 | { | 303 | { |
285 | if (bookmark_get_playlist_and_track(bookmark, &pl_start, &pl_end, &track)) | 304 | |
305 | if (bookmark_get_playlist_and_track_hash(bookmark, &pl_hash, &pl_track_hash)) | ||
286 | { | 306 | { |
287 | comp_playlist = true; | 307 | comp_playlist = true; |
288 | pl_len = pl_end - pl_start; | 308 | comp_track = (global_settings.usemrb == BOOKMARK_ONE_PER_TRACK); |
289 | if (global_settings.usemrb == BOOKMARK_ONE_PER_TRACK) | ||
290 | comp_track = true; | ||
291 | } | 309 | } |
292 | } | 310 | } |
311 | |||
293 | logf("adding bookmark to %s [%s]", fnamebuf, bookmark); | 312 | logf("adding bookmark to %s [%s]", fnamebuf, bookmark); |
294 | /* Writing the new bookmark to the begining of the temp file */ | 313 | /* Writing the new bookmark to the begining of the temp file */ |
295 | write(temp_bookmark_file, bookmark, strlen(bookmark)); | 314 | write(temp_bookmark_file, bookmark, strlen(bookmark)); |
296 | write(temp_bookmark_file, "\n", 1); | 315 | write(temp_bookmark_file, "\n", 1); |
297 | bookmark_count++; | 316 | bookmark_count++; |
298 | 317 | ||
318 | /* WARNING underlying buffer to *bookmrk gets overwritten after this point! */ | ||
319 | |||
299 | /* Reading in the previous bookmarks and writing them to the temp file */ | 320 | /* Reading in the previous bookmarks and writing them to the temp file */ |
300 | logf("opening old bookmark %s", bookmark_file_name); | 321 | logf("opening old bookmark %s", bookmark_file_name); |
301 | bookmark_file = open(bookmark_file_name, O_RDONLY); | 322 | bookmark_file = open(bookmark_file_name, O_RDONLY); |
@@ -315,15 +336,14 @@ static bool add_bookmark(const char* bookmark_file_name, | |||
315 | equal = false; | 336 | equal = false; |
316 | if (comp_playlist) | 337 | if (comp_playlist) |
317 | { | 338 | { |
318 | if (bookmark_get_playlist_and_track(global_temp_buffer, | 339 | if (bookmark_get_playlist_and_track_hash(global_temp_buffer, |
319 | &bm_pl_start, &bm_pl_end, &bm_track)) | 340 | &bm_pl_hash, &bm_pl_track_hash)) |
320 | { | 341 | { |
321 | bm_pl_len = bm_pl_end - bm_pl_start; | 342 | equal = (pl_hash == bm_pl_hash); |
322 | equal = (pl_len == bm_pl_len) && | ||
323 | !strncmp(pl_start, bm_pl_start, pl_len); | ||
324 | |||
325 | if (equal && comp_track) | 343 | if (equal && comp_track) |
326 | equal = !strcmp(track, bm_track); | 344 | { |
345 | equal = (pl_track_hash == bm_pl_track_hash); | ||
346 | } | ||
327 | } | 347 | } |
328 | } | 348 | } |
329 | if (!equal) | 349 | if (!equal) |
@@ -339,11 +359,6 @@ static bool add_bookmark(const char* bookmark_file_name, | |||
339 | } | 359 | } |
340 | close(temp_bookmark_file); | 360 | close(temp_bookmark_file); |
341 | 361 | ||
342 | /* only retrieve the path*/ | ||
343 | open_temp_bookmark(fnamebuf, | ||
344 | sizeof(fnamebuf), | ||
345 | O_PATH, | ||
346 | bookmark_file_name); | ||
347 | remove(bookmark_file_name); | 362 | remove(bookmark_file_name); |
348 | rename(fnamebuf, bookmark_file_name); | 363 | rename(fnamebuf, bookmark_file_name); |
349 | 364 | ||