On 09/15/2013 04:53 PM, Jakub Jermar wrote:
> For the record, I was watching the changes closely until about two weeks
> or so ago and I quite liked them. The whole series starts with a nice
> cleanup/handlification of the interface and the code and it seemed
> reasonable to me. Now, I still need to look at what was done in the most
> recent changes.
Ok, I went through the changes as they stand now. It is possible that if
you throw the old API away again, new issues will arise, but this is a
list of reservations or comments that I have now:
- use of magic constants, such as 4096 in valid_ptr() and formerly(?)
also >= 16 for valid handles
- use of __builtin's
- async_exchange_begin(), does not require you to test for NULL return
value; as I was told at the camp, the exchange-consuming functions
should handle this gracefully; for example in vfs_connect_internal()
- new synchronization primitives (such as in sleeper.h) (I guess Martin
already pointed this out) and fancy synchronization using atomics
instead of simple lock-based synchronization using the already
existing stuff (already pointed out by me :-)
- use of printf() formatters; instead of using a portable formatter, you
appear to cast the variable to match the formatter; I found the
following cases:
(long long unsigned) service_id
(long long) de->size
http://trac.helenos.org/wiki/Printf could help
- inbox_entry implements its own linked list; why not use the already
existing list_t and its list_first() and list_next() to traverse the
list in your desired way?
Jakub
_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/listinfo/helenos-devel
