Tighten up checking by adding length checks to loops which
ensure the iteration stays within bounds. Check is disabled
by default and can be enabled using a BUFLIB_PARANOIA bit.
Change-Id: I35e911e0878797d5ebf732be548ca659f6910fe0
Buflib is written with a lot of hardcoded offsets to header fields,
arbitrary pointer arithmetic, and similar but not quite duplicated
code, making maintenance a nightmare.
Most of the pointer arithmetic involving header fields is replaced
by indexing from two well-defined pointers, the block start and end
pointers. The start pointer points to the first header field, and
he end pointer is one past the end of the header.
Hardcoded field indices are replaced by two enums. Forward indices
(fidx_XXX) are used to access fields from a block start pointer and
negated backward indices (-bidx_XXX) are used to index from a block
end pointer. There is no overlap between the indices because of the
variable length name field in the middle of the header. The length
of the fixed fields in the block header is now a #define'd constant
rather than being open coded.
There is now a function to acquire the block end pointer from the
user data pointer (ie. the pointer stored in the handle table). The
old code was not consistent in this; some functions would handle a
non-aligned user pointer, which may occur as a result of shrinking,
while other uses just assumed the user pointer was aligned.
Block CRC calculations have also been factored out to a function
that accepts block start and end pointers.
Change-Id: I6a7e8a8c58aec6c6eaf0e5021400032d8e5f841e
The block header has a variable length due to the embedded name.
The name length is stored at the back of the header after the
name, in order to allow finding the start of the header if only
the user data pointer is known (eg. from the handle table).
The name length is actually not interesting in itself; storing
the total length of the block header instead is marginally more
efficient, saving one addition in handle_to_block().
Instead the extra arithmetic must be done by buflib_get_name(),
which is a much less common operation than handle_to_block().
Change-Id: Ia339a1d2f556a11a49deae0871203e70548bd234
The way it iterated over the handle table is unsafe if *every*
handle is free, leading to an out of bounds access.
This is a contrived example, but the bug can be triggered by
making several allocations, freeing them out of order so that
the handle table remains uncompacted, and then triggering a
compaction using buflib_alloc_maximum().
Change-Id: I879e2f0b223e6ca596769610ac46f4edf1107f5c
It isn't obvious why the "handle->val = -1" at the end of handle_alloc
is needed so add an explanatory comment. But "handle->val = 1" prior to
calling handle_free is simply a dead store.
Change-Id: I9ab4e96e87e940cfd1a2ed56f089287821669c73
Since we're scanning the handle table for the first free slot,
we know none of the scanned slots are free. Use that knowledge
to update first_free_handle and avoid rescanning filled slots
again when the next handle is allocated.
Change-Id: I457372f66c231168cfffa7e905d1e9fb80002f5f
Use the standard IS_ALIGNED macro and check alignment against
sizeof(buflib_data), as it can be greater than 4 bytes if we're
on a 64-bit platform (eg. simulator).
Change-Id: I15110937d1f2978e733d169050de9531fe218214
Exiting the loop implies next_block == block, so remove that check.
The check ret < block is false only if block is the first block, which
can be checked before the loop, saving a few cycles in that case.
Change-Id: Id493b5259a23a35a70b09dfe4bc4eacaf420760c
There are various allocations that can't be moved or shrunk.
Provide a global callback struct for this use case instead of
making each caller declare its own dummy struct.
Also fixed ROLO and x1000 installer code which incorrectly
used movable allocations.
Change-Id: I00088396b9826e02e69a4a33477fe1a7816374f1
when buflib_init is called with a buffer smaller than
sizeof(union buflib_data); size will be zero
Later when the alloc fails buflib will keep try to free items
in order to satisify the request this crashes in the sim
I suspect this behavior holds true on device as well
but I havent verified this as of yet.
patch adds minimal overhead to the buflib and panics when the size is too small
Change-Id: I46e510367fc1cac19ce01ee6f92d8cf0d65ef914
The documentation of buflib first mentions metadata
and then changes to "cookie" without explaining it.
Fix it by sticking to metadata.
Change-Id: I0b36b18f4f2590132901c10326481975f8b9b9da
Document the fact that buffers are movable by default.
Care must be taken to not pass them to functions that yield().
Also clarify other things:
- Passing NULL as "ops" to buflib_alloc_ex() causes
buffers to be movable by default (but not shrinkable).
- If you want shrinkable buffers during compaction,
you have to provide a shrink callback.
- To disable buffer movement, you have to pass NULL
for the move_callback inside the callback structure.
- The concept of default callbacks was removed
long ago, remove the only reference of it.
Change-Id: I3bf0ea6b08b507d80a19f3c2c835aca32b3f7800
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
This allows buflib clients to more accurately estimate the total memory usage.
It's still not 100% accurate because the handle table grows in blocks, thus
buflib might use more memory that caused by allocations directly.
Change-Id: I68338bb94f510ad188fcb588aebf895b5f9197c5
This should catch the case of buffer misuse which results
in corrupted cookie of next allocation. The check is performed
on move_block() so it may be a bit late.
There is buflib_check_valid() provided which checks the
integrity of all cookies for given context.
On DEBUG build with --sdl-thread this check is carried out
for core_ctx on every context switch to catch problems earlier.
Change-Id: I999d4576084592394e3dbd3bdf0f32935ff5f601
Reviewed-on: http://gerrit.rockbox.org/711
Reviewed-by: Thomas Martitz <kugel@rockbox.org>
This function relocates a buflib back buffer, updating pointers in struct
buflib_context. It does not move any data by itself.
The intended use-case is buflib-on-buflib, where a buflib back buffer is
allocated with buflib and attempted to be moved. The move_callback() can call
this and return BUFLIB_CB_OK on success. No move_callback() is called for the
subordinate buflib buffer, therefore it must not contain non-movable
allocations. The caller is generally responsible moving the data and all its
implications.
Change-Id: I869219f9cff786a172c9e917a5f34470073892e6
This function will now ask shrinkable allocations to give up all of their
memory. With future support of playback.c this can be used as a safe
replacement for audio_get_buffer().
Change-Id: I290a51d2c75254e66baf5698c41dc444dea6247a
buflib_allocatable() is what buflib_available() was before (it was in fact
simply renamed). It returns the largest contiguous block of memory. This
can be allocated and will definitely succeed, although larger allocations
may also succeed if the buffer can be compacted and shrinked.
buflib_available() now counts all free bytes, contiguous or not. This
better matches the description and how the caller use it.
Change-Id: I511e4eb5f4cf1821d957b3f4ef8a685ce40fe289
Reviewed-on: http://gerrit.rockbox.org/481
Reviewed-by: Thomas Martitz <kugel@rockbox.org>
Tested-by: Thomas Martitz <kugel@rockbox.org>
It can cause excessively long interrupt outages if moving a larger
buffer and disrupt audio where DMA is not at a higher interrupt priority
such as FIQ.
Some targets, like Gigabeat S, have very low audio interrupt latency
requirements and will even channel swap if they are missed. Pictureflow
will make the issue very obvious. Even then, moves could take
milliseconds or more depending on the buffer size which is far too long
for any target.
Change-Id: I8e7817213e901da67c36b7eb25d7cb1c1e3ba802
Reviewed-on: http://gerrit.rockbox.org/472
Reviewed-by: Michael Sevakis <jethead71@rockbox.org>
Tested-by: Michael Sevakis <jethead71@rockbox.org>
Buffers are not allocated and thread is not created until the first
call where voice is required.
Adds a different callback (sync_callback) to buflib so that other
sorts of synchonization are possible, such as briefly locking-out the
PCM callback for a buffer move. It's sort of a messy addition but it
is needed so voice decoding won't have to be stopped when its buffer
is moved.
Change-Id: I4d4d8c35eed5dd15fb7ee7df9323af3d036e92b3
* shrinking now considers freespace just before the alloc-to-be-shrinked,
that means less (or sometimes none at all) is taken from the audio buffer.
* core_available() now searches for the best free space, instead of simply the end,
i.e. it will not return 0 if the audio buffer is allocated and there's free space
before it. It also runs a compaction to ensure maximum contiguous memory.
audio_buffer_available() is also enhanced. It now considers the 256K reserve buffer,
and returns free buflib space instead if the audio buffer is short.
This all fixes the root problem of FS#12344 (Sansa Clip+: PANIC occurred when
dircache is enabled), that alloced from the audio buffer, even if it was very
short and buflib had many more available as free space before it.
git-svn-id: svn://svn.rockbox.org/rockbox/trunk@31006 a1c6a512-1295-4272-9138-f99709370657
This makes accessing the buffers with core_get_data() from
interrupt context safe, other buflib functions aren't really safe (yet).
git-svn-id: svn://svn.rockbox.org/rockbox/trunk@30736 a1c6a512-1295-4272-9138-f99709370657
Handles are allocated at the end, growing downwards.
The tiny allocations from r30478 broke buflib, since it was assumed that the only shrinkable allocation (the audiobuffer) is the very last allocation.
The tiny allocations however fit into the reserve buffer for new handles, breaking the above assumption, and they can't shrink to make room for handles.
Now, move any allocations before audiobuf (or shrink audiobuf like before) to make room for handles. This also unifies some duplicated code.
git-svn-id: svn://svn.rockbox.org/rockbox/trunk@30486 a1c6a512-1295-4272-9138-f99709370657
* Enhance allocation function comments to better state the return value and what an invalid value is
* Change clients to check for "< 0" instead of "<= 0" or "== 0"
* Return -1 or -2 depending on the exact failure in buflib_alloc_ex.
git-svn-id: svn://svn.rockbox.org/rockbox/trunk@30469 a1c6a512-1295-4272-9138-f99709370657
This enables the ability to allocate (and free) memory dynamically
without fragmentation, through compaction. This means allocations can move
and fragmentation be reduced. Most changes are preparing Rockbox for this,
which many times means adding a move callback which can temporarily disable
movement when the corresponding code is in a critical section.
For now, the audio buffer allocation has a central role, because it's the one
having allocated most. This buffer is able to shrink itself, for which it
needs to stop playback for a very short moment. For this,
audio_buffer_available() returns the size of the audio buffer which can
possibly be used by other allocations because the audio buffer can shrink.
lastfm scrobbling and timestretch can now be toggled at runtime without
requiring a reboot.
git-svn-id: svn://svn.rockbox.org/rockbox/trunk@30381 a1c6a512-1295-4272-9138-f99709370657
The buflib memory allocator is handle based and can free and
compact, move or resize memory on demand. This allows to effeciently
allocate memory dynamically without an MMU, by avoiding fragmentation
through memory compaction.
This patch adds the buflib library to the core, along with
convinience wrappers to omit the context parameter. Compaction is
not yet enabled, but will be in a later patch. Therefore, this acts as a
replacement for buffer_alloc/buffer_get_buffer() with the benifit of a debug
menu.
See buflib.h for some API documentation.
git-svn-id: svn://svn.rockbox.org/rockbox/trunk@30380 a1c6a512-1295-4272-9138-f99709370657
