summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Jarosch <tomj@simonv.com>2015-01-02 01:06:06 +0100
committerThomas Jarosch <tomj@simonv.com>2015-01-02 01:11:54 +0100
commit726537508737351d028c6730d30d9ec38fa34e4e (patch)
tree164a21a8aaee0ac2f868196b5d6c7433126a04e3
parent9076b433d18b5db1a1987fe99ca7c70808f22b0e (diff)
downloadrockbox-7265375.tar.gz
rockbox-7265375.tar.bz2
rockbox-7265375.zip
Shortcuts: Add move callback for buflib allocations
If we don't provide a callback to buflib_alloc(), the buffer is always movable (to reduce fragmentation). We were passing around buffers to multiple functions that call yield() and might trigger buflib compaction. -> add locking while we are working on the buffers. Also added source code comments that explain why we added the locking in that particular section. Change-Id: Ie32867b0b735ddb2905fd4bd51342f61035f836f
-rw-r--r--apps/shortcuts.c54
1 files changed, 48 insertions, 6 deletions
diff --git a/apps/shortcuts.c b/apps/shortcuts.c
index 1153edd2ad..9abd82af2f 100644
--- a/apps/shortcuts.c
+++ b/apps/shortcuts.c
@@ -83,6 +83,22 @@ struct shortcut_handle {
static int first_handle = 0;
static int shortcut_count = 0;
+static int buflib_move_lock;
+static int move_callback(int handle, void *current, void *new)
+{
+ (void)handle;
+ (void)current;
+ (void)new;
+
+ if (buflib_move_lock > 0)
+ return BUFLIB_CB_CANNOT_MOVE;
+
+ return BUFLIB_CB_OK;
+}
+static struct buflib_callbacks shortcut_ops = {
+ .move_callback = move_callback,
+};
+
static void reset_shortcuts(void)
{
int current_handle = first_handle;
@@ -97,17 +113,18 @@ static void reset_shortcuts(void)
}
first_handle = 0;
shortcut_count = 0;
-}
+}
static struct shortcut* get_shortcut(int index)
{
int handle_count, handle_index;
int current_handle = first_handle;
struct shortcut_handle *h = NULL;
-
+
if (first_handle == 0)
{
- first_handle = core_alloc("shortcuts_head", sizeof(struct shortcut_handle));
+ first_handle = core_alloc_ex("shortcuts_head",
+ sizeof(struct shortcut_handle), &shortcut_ops);
if (first_handle <= 0)
return NULL;
h = core_get_data(first_handle);
@@ -126,7 +143,10 @@ static struct shortcut* get_shortcut(int index)
{
char buf[32];
snprintf(buf, sizeof buf, "shortcuts_%d", index/SHORTCUTS_PER_HANDLE);
- h->next_handle = core_alloc(buf, sizeof(struct shortcut_handle));
+ /* prevent invalidation of 'h' during compaction */
+ ++buflib_move_lock;
+ h->next_handle = core_alloc_ex(buf, sizeof(struct shortcut_handle), &shortcut_ops);
+ --buflib_move_lock;
if (h->next_handle <= 0)
return NULL;
h = core_get_data(h->next_handle);
@@ -190,12 +210,17 @@ static void shortcuts_ata_idle_callback(void)
char buf[MAX_PATH];
int current_idx = first_idx_to_writeback;
int append = overwrite_shortcuts ? O_TRUNC : O_APPEND;
-
+
if (first_idx_to_writeback < 0)
return;
fd = open(SHORTCUTS_FILENAME, append|O_RDWR|O_CREAT, 0644);
if (fd < 0)
return;
+
+ /* ensure pointer returned by get_shortcut()
+ survives the yield() of write() */
+ ++buflib_move_lock;
+
while (current_idx < shortcut_count)
{
struct shortcut* sc = get_shortcut(current_idx++);
@@ -221,6 +246,7 @@ static void shortcuts_ata_idle_callback(void)
reset_shortcuts();
shortcuts_init();
}
+ --buflib_move_lock;
first_idx_to_writeback = -1;
}
@@ -336,15 +362,25 @@ void shortcuts_init(void)
fd = open_utf8(SHORTCUTS_FILENAME, O_RDONLY);
if (fd < 0)
return;
- first_handle = core_alloc("shortcuts_head", sizeof(struct shortcut_handle));
+ first_handle = core_alloc_ex("shortcuts_head", sizeof(struct shortcut_handle), &shortcut_ops);
if (first_handle <= 0)
return;
h = core_get_data(first_handle);
h->next_handle = 0;
+
+ /* we enter readline_cb() multiple times with a buffer
+ obtained when we encounter a "[shortcut]" section.
+ fast_readline() might yield() -> protect buffer */
+ ++buflib_move_lock;
+
fast_readline(fd, buf, sizeof buf, &param, readline_cb);
close(fd);
if (param && verify_shortcut(param))
shortcut_count++;
+
+ /* invalidate at scope end since "param" contains
+ the shortcut pointer */
+ --buflib_move_lock;
}
static const char * shortcut_menu_get_name(int selected_item, void * data,
@@ -462,6 +498,10 @@ int do_shortcut_menu(void *ignored)
}
push_current_activity(ACTIVITY_SHORTCUTSMENU);
+ /* prevent buffer moves while the menu is active.
+ Playing talk files or other I/O actions call yield() */
+ ++buflib_move_lock;
+
while (done == GO_TO_PREVIOUS)
{
if (simplelist_show_list(&list))
@@ -540,5 +580,7 @@ int do_shortcut_menu(void *ignored)
}
}
pop_current_activity();
+ --buflib_move_lock;
+
return done;
}