On Sun, Feb 22, 2015 at 08:02:29AM -0600, Gary Mills via illumos-developer
wrote:
> One thing we can change in illumos now is the way it handles different
> architectures. I'm not talking about architecture-specific
> directories, which are better in many cases, but conditional
> compilation. The current paradigm is like this:
>
> #ifdef SPARC
> /* SPARC code */
> #else
> /* x86 code */
> #endif
Yes, this is terrible.
> That's difficult to extend. It could be changed to something like
> this:
>
> #if defined(SPARC)
> /* SPARC code */
> #elif defined(X86)
> /* x86 code */
> #else
> #error "Unknown architecture"
> #endif
While this is better, I think there are even better ways. Either version of
the above code checks for ISA/platform. These should really be
feature-checks. E.g.,
#if defined(PLATFORM_HAS_FOO)
/* code to handle foo */
#else
ret = ENOTSUP;
#endif
The PLATFORM_HAS_FOO can live in one of the several params header file which
we already have. Then, when dealing with a new platform/architecture, it's
a matter of enabling bits and pieces via a header file. Sure, there'll be
more work during bringup, but I'd argue there will be less time wasted and
the code will be more readable.
In the case some optional interfaces, we could even do something to
completely eliminate the #ifdef mess. E.g.,
in genunix:
#pragma weak do_foo = generic_do_foo
int generic_do_foo()
{
return (ENOTSUP);
}
in unix on platforms that can do "foo":
int do_foo()
{
/* code to do foo */
}
and finally, the rest of the system can call do_foo without having to worry
whether or not the specific platform has it:
ret = do_foo();
if (ret != 0)
goto err;
...
Of course, some combination of the two is probably the cleanest.
Here's an example that's not made up. Recently, libproc learned how to
process Linux core files. The functionality got gated with x86 defines
because only x86 Linux core files are supported. If we added support for
sparcv9 (but not earlier) Linux core files, we'd have to hunt down all the
places where "x86" really means "Linux core file support" and change them.
If instead of x86 check these were PLATFORM_HAS_LINUX_CORE_FILE_SUPPORT,
then aside from actually implementing the sparc pieces, it'd be a matter of
throwing one line in a params.h and suddenly relevant pieces of code would
be enabled.
Note: I'm *not* suggesting that we make Illumos configurable. I'm
suggesting that we try to take more feature-based approach to conditionally
compiled code.
Jeff.
--
Evolution, n.:
A hypothetical process whereby infinitely improbable events occur with
alarming frequency, order arises from chaos, and no one is given credit.
-------------------------------------------
smartos-discuss
Archives: https://www.listbox.com/member/archive/184463/=now
RSS Feed: https://www.listbox.com/member/archive/rss/184463/25769125-55cfbc00
Modify Your Subscription:
https://www.listbox.com/member/?member_id=25769125&id_secret=25769125-7688e9fb
Powered by Listbox: http://www.listbox.com
