Am 22.02.2015 um 21:15 schrieb Thomas Jarosch:
Hi Thomas (kugel),

Am Sonntag, 15. Februar 2015, 23:45:48 schrieb Thomas Martitz:
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 <t...@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
thanks for the review. I noticed the same thing a few days after
committing it. It occupied my mind why it wasn't crashing before ;)
Do you think we should revert it or keep it? It won't hurt I guess.

Did you have a chance to look at the other buflib changes I did?
They should be correct IMHO.


IMO, yes, it should be reverted, because adds unecessary overhead and gives the false impression that callbacks are always required.

I didn't notice other problems with your buflib related commits but I maybe missed some. Do you want me to look at a particular change?

Best regards.

Reply via email to