From: Thomas Martitz <kugel_at_rockbox.org>
Date: Sun, 15 Feb 2015 23:45:48 +0100

The below commit isn't necessary.

buflib buffers can be passed to yielding functions just fine. Problems
only arise if the are concurrent allocations, for example if two threads
allocate from the same context simultaneously or if the callee does it's
own allocations. This can't happen in the pictureflow case, it has it's
own context and a single thread allocating from it.

Therefore the problem isn't yield() itself, but possible concurrent
buflib_alloc() calls that result from the thread switch. This is because
compaction only ever happens on allocation (and not in a backgroud
thread or so).

Best regards.


commit 9076b433d18b5db1a1987fe99ca7c70808f22b0e
Author: Thomas Jarosch <tomj_at_simonv.com>
Date: Thu Jan 1 23:45:24 2015 +0100

     PictureFlow: Add move callback for buflib allocations

     If we don't provide a callback to buflib_alloc(),
     the buffer is always movable (to reduce fragmentation).

     Since we pass our buffer to functions that call yield(),
     this could lead to memory corruption on buflib compaction.

     Change-Id: Id1fad1822479d692551c55cb8bc87cea7b78f759

diff --git a/apps/plugins/pictureflow/pictureflow.c
b/apps/plugins/pictureflow/pictureflow.c
index 53fede1..afe108b 100644
--- a/apps/plugins/pictureflow/pictureflow.c
+++ b/apps/plugins/pictureflow/pictureflow.c
_at__at_ -443,6 +443,18 _at__at_ static int track_list_visible_entries = 0;
  static int track_list_y;
  static int track_list_h;

+static int locked_buflib_handle;
+static int move_callback(int handle, void *current, void *new)
+{
+ (void)current; (void)new;
+ if (handle == locked_buflib_handle)
+ return BUFLIB_CB_CANNOT_MOVE;
+ return BUFLIB_CB_OK;
+}
+static struct buflib_callbacks pictureflow_ops = {
+ .move_callback = move_callback,
+};
+
  /*
      Proposals for transitions:

_at__at_ -1534,7 +1546,7 _at__at_ static int read_pfraw(char* filename, int prio)

      int hid;
      do {
- hid = rb->buflib_alloc(&buf_ctx, size);
+ hid = rb->buflib_alloc_ex(&buf_ctx, size, "PictureFlow",
&pictureflow_ops);
      } while (hid < 0 && free_slide_prio(prio));

      if (hid < 0) {
_at__at_ -1544,6 +1556,7 _at__at_ static int read_pfraw(char* filename, int prio)

      rb->yield(); /* allow audio to play when fast scrolling */
      struct dim *bm = rb->buflib_get_data(&buf_ctx, hid);
+ locked_buflib_handle = hid;

      bm->width = bmph.width;
      bm->height = bmph.height;
_at__at_ -1555,6 +1568,7 _at__at_ static int read_pfraw(char* filename, int prio)
          rb->read( fh, data , sizeof( pix_t ) * bm->width );
          data += bm->width;
      }
+ locked_buflib_handle = -1;
      rb->close( fh );
      return hid;
  }
_at__at_ -1709,6 +1723,7 _at__at_ static inline struct dim *get_slide(const int hid)
      struct dim *bmp;

      bmp = rb->buflib_get_data(&buf_ctx, hid);
+ locked_buflib_handle = hid;

      return bmp;
  }
_at__at_ -2100,6 +2115,9 _at__at_ static void render_all_slides(void)
      if (step != 0 && num_slides <= 2) /* fading out center slide */
          alpha = (step > 0) ? 256 - fade / 2 : 128 + fade / 2;
      render_slide(&center_slide, alpha);
+
+ /* free up lock on last used slide */
+ locked_buflib_handle = -1;
  }


diff --git a/firmware/buflib.c b/firmware/buflib.c
index 9ee721a..a47f0a0 100644
--- a/firmware/buflib.c
+++ b/firmware/buflib.c
_at__at_ -677,7 +677,7 _at__at_ buflib_free(struct buflib_context *ctx, int handle_num)
      else
      {
      /* Otherwise, set block to the newly-freed block, and mark it
free, before
- * continuing on, since the code below exects block to point to a free
+ * continuing on, since the code below expects block to point to a free
       * block which may have free space after it.
       */
          block = freed_block;
Received on 2015-02-15