On Thu, 2017-01-12 at 10:51 +0000, Thomas Preudhomme wrote:
My bad. I've rebased it on latest master branch. Please find the
patch attached.
Thanks! Now it builds fine (;
In my very limited testing it seems to work very nicely and the idea of
using "struct _lock;" is very clever, as it allows me to do the
retargeting in a very clean way:
--- >8 --- >8 --- >8 --- >8 --- >8 --- >8 --- >8 --- >8 ---
struct _lock : public distortos::Mutex
{
using Mutex::Mutex;
};
_lock _lock___sinit_lock {distortos::Mutex::Type::recursive,
distortos::Mutex::Protocol::priorityInheritance};
_lock _lock___sfp_lock {distortos::Mutex::Type::recursive,
distortos::Mutex::Protocol::priorityInheritance};
_lock _lock___atexit_lock {distortos::Mutex::Type::recursive,
distortos::Mutex::Protocol::priorityInheritance};
_lock _lock___malloc_lock_object {distortos::Mutex::Type::recursive,
distortos::Mutex::Protocol::priorityInheritance};
void __retarget_lock_init (_LOCK_T *lock)
{
*lock = new _lock {distortos::Mutex::Type::normal,
distortos::Mutex::Protocol::priorityInheritance};
}
void __retarget_lock_close(_LOCK_T lock)
{
delete lock;
}
void __retarget_lock_acquire (_LOCK_T lock)
{
lock->lock();
}
void __retarget_lock_release (_LOCK_T lock)
{
lock->unlock();
}
...
--- >8 --- >8 --- >8 --- >8 --- >8 --- >8 --- >8 --- >8 ---
But I still see a big issue with the weak objects that you provided in
lock.c... The problem here is that now the user can redefine just 3 of
them, so the last one will be taken from the weak definition that you
provided. There will be no error/warning during compilation or linking.
This will of course fail terribly during run-time due to mismatched
types... While this may seem a bit unlikely at the first glance, the
possible scenario that would lead to that is:
1. today newlib adds this very cool feature and adds weak objects for
all used locks.
2. after some time RTOS developers start to support it and also provide
their own versions of the locks needed by newlib.
3. after some more time newlib adds another lock somewhere, along with
the object.
At this moment all users of code created during step 2 linked with
newlib version from step 3 will result in undefined behaviour whenever
the new lock is used...
You also missed some of the locks that are used by newlib:
newlib/libc/posix/telldir.c:__LOCK_INIT(static, dd_hash_lock);
newlib/libc/time/tzlock.c:__LOCK_INIT(static, __tz_lock_object);
newlib/libc/stdlib/quick_exit.c:__LOCK_INIT(static, atexit_mutex);
newlib/libc/stdlib/envlock.c:__LOCK_INIT_RECURSIVE(static, __env_lock_object);
Maybe it would be somehow possible to bind all of the objects (and
maybe even functions) together, so that when user provides just one
definition, all other weak definitions will be immediately disabled?
This way the user would either have to provide all or none, without an
option to only provide some of the objects/functions. This is my only
concern - I have nothing against providing weak definitions, but the
possibility of overriding only some of the objects is a bit disturbing.