On Mon, 17 Dec 2007 18:17:35 +0100 Rafal Jaworowski raj@semihalf.com wrote:
Haavard Skinnemoen wrote:
I'm sorry, but I really hate this stuff.
- What happens if changes to the API is needed? Will be keep adding new "system calls" every time a new limitation with the existing interface is found (like Linux does)?
The API is versioned and consumer code is able to verify it. When we need to change/extend it, the version is bumped: that's similar how many other APIs are managed, like UNIX libs.
So old versions of the interface must be kept around in case legacy applications need them (that's how solib versioning works, IIRC.) Or do applications have to anticipate that the interface may change in a future version?
- In other words, do we really _want_ a "stable API"?
Depending on the definition of "we", yes :-) Besides that we need a stable API for FreeBSD purposes, I'm sure U-Boot as a whole benefits from improvement in this area.
Yeah, I understand that you want to use this with FreeBSD somehow. But I've never seen a good answer to _why_ you need to do it this way, apart from something along the lines of "because we can", or "we want to turn u-boot into a full-fledged OS".
- What's the rationale for the "syscalls" this patch exports, and are you absolutely sure the prototypes are sane? If you want a stable API, you need to get it right the first time.
Per earlier explanation given by Marcel, the main purpose of these 'syscalls' is providing a well defined and stable interface to access services that U-Boot implements like operations on the console, networking, storage etc. The prototypes we have now have been thought out for a while and reviewed, and I think they are sane.
Having the boot loader provide "advanced" services to the OS (console, storage, etc.) has been done before. It's called "BIOS", and it absolutely _sucks_. Please don't turn u-boot into a BIOS...
- Both the API core and the examples are littered with external declarations. Can we please put such things in header files where it belongs?
There's a couple of extern declarations that indeed could be placed in a separate header, but it's usually fine balance when to put something into a separate file (and bloat the files structure..), and in this case I decided not to for simplicity. All other externs are for accessing existing U-Boot objects.
"A couple"? There are sh*tloads of them.
As for accessing existing U-Boot objects, that's not an excuse. If a global function is missing a corresponding header declaration, it should be added.
I've been going around trying to get rid of some global definitions and turning them into static ones, assuming that if they don't exist in a header, they aren't used. The problems usually show up a couple of hours into the final compile-test...
- All syscalls are implemented as vararg functions, so it's difficult to tell what arguments they take and whether or not they are being used correctly from the other side of the "syscall" line. A standard set of wrappers and associated header files would help, of course.
There is a pseudo-signature description in the comment for each syscall that was meant to help and document. Also, the helper wrapper you mention is already there: it's the glue layer, which implements front-end conveniency calls the consumer can use, but it's not mandatory and syscall can be invoked directly.
Yeah, but the stubs have no associated header file, so you have to declare them yourself. That's just begging for fun-to-debug problems where the caller and the callee have different opinions about the function signature...even more fun when the problems only show up on certain architectures.
This is a general problem with the existing u-boot code, btw. I don't dare even guess how many hidden bugs there are all around the tree because of function prototype mismatches...
- How is this really different from the existing jumptable stuff? It looks like it's just a different set of exported functions. Will the crufty old jumptable interface be removed at some point? Presumably, the new interface is superior, so it should be. Right?
The new interface is a similar mechanism, but it does not entangle external application with configuration of U-Boot it happens to run upon. It's meant to be more robust and extensible, but it is still initial implementation. This is why it's optional so might be treated as experimental feature right now, but my hope is after U-Boot-FreeBSD integration settles down, the old jumptable approach can be retired (I volunteer to convert legacy standalone U-Boot apps to the new scheme at that point).
Well, if it's going to _replace_ the old interface, it all sounds a bit more reasonable.
Haavard