On Wed, Jul 17, 2013 at 6:40 PM, Glenn Fowler <[email protected]> wrote:
>
> On Sat, 29 Jun 2013 00:37:11 +0200 Cedric Blancher wrote:
>> 2013/6/28 Glenn Fowler <[email protected]>:
>> >
>> > the AT&T Software Technology ast alpha 2013-06-28 source release
>> > has been posted to the download site
>> >         http://www.research.att.com/sw/download/alpha/
>> > the package names and md5 checksums are
>> >             INIT  eddbf89d061348519d86f2618b708a94
>> >         ast-base  a745a7d4ce6f53c2e4134af4cc835ff7
>> >         ast-open  fdb74839ff041e34c800c333188a050e
>> >          ast-ksh  8f22428cf30af7146bd210664c2fd166
>> > the md5 sums should match the ones listed on the download page
>
>> The release is unusable. The new "API" - if it can be called like that
>> - added wrappers to all syscalls via #define, which breaks down on
>> OpenBSD or other platforms which already use #defines for security
>> wrappers. It's also undebuggable by adding yet another layer of hidden
>> complexity. I wouldn't mind if if the code would call _ast_open() and
>> friends directly but hiding it via #define open _ast_open collides
>> with too many other things, including system libraries and the ability
>> of normal minds to grok it.
>
>> So this won't fly.
>
> thanks for the feedback
>
> unfortunately we don't have acces to bsd machines anymore
> bsd *never* did headers right
> e.g., if posix says
>         #include <foo.h>
> bsd takes it on itself to demand
>         #include <sys/hack.h>
>         #include <sys/hackier.h>
>         #include <foo.h>
> so I'm not surprised that we hit macro clashes
> send me offlist the files named by
>         bin/package results path
> and if you did more than one build
>         bin/package results path old
>
> we knew the intercepts would be controversial, especially the varargs ioctl()
> but this is the best way we could think of to flesh out EINTR problems
> that arose from the recent signal/queue storm tests
> as far as we can tell few system calls in all ast libraries and commands are
> immune from EINTR error returns, including surprising ones like close() and 
> stat()
>
> there is no way we could do the edit to wrap syscalls with restart logic,
> possibly just for debugging purposes, in a timely manner
> so we did it by default for all ast code via macro black magic
> knowing that we may run afoul of others doing similar black magic
> as the problems arise we'll address them
> for now the default is to always intercept
> but there is a way to build with intercepts disabled
>         -D_AST_INTERCEPT=0
> *but don't do this for ast code*
> in the future the default could change
>
> the intercept approach addresses many issues
> * adding restart logic, macros or not, to every piece of ast code would be 
> fugly
>   and I would not like editing, debugging or maintaining that code on a daily 
> basis
>   so please don't submit patches to restartify ast code
> * whos to say some other issues like EINTR won't arise tomorrow -- with 
> intercepts
>   we may have a much easier pathway to address those issues
> * any user code that expects to be used in ksh builtins or ast plugins must
>   do the restart logic -- its much easier to instruct builtin developers to
>   "#include <ast.h>" than to "wrap each syscall with foomacro() barmacro()"
>   and the latter would have to be bullet proof -- not that easy to say months 
> later
>   "oops, we should have said foomacro(special arg) barmacro(another special 
> arg)";
>   if the user code doesn't do syscall macro intercepts it should go smoothly,
>   otherwise the users will have to "-D_AST_INTERCEPT=0" and examine the user
>   or 3rd party code for EINTR restartedness

My point is: How is restart controlled? Is this going to be a global
option or thread-local? If it's going to be thread-local you will have
to do context switches between library boundaries, i.e. library a does
it's own ast restart settings and calls library b which does it's own
ast restart settings. Which means each call needs code to save and
restore the state.

This sounds simple, yes? Yes, it is simple. At the beginning. For a
small project like 'hello world'.
Unfortunately - for big projects - it isn't simple. Netscape 4 was
such an example where the good intentions "making it easy and use
save/restore" paved a way for going from 5 save/restore calls between
module boundaries up to over 400 in Netscape 4.5. This design is
nowadays taught in university programming schools as "context switch
way to hell", a cautious tale of what doomed the whole Netscape 4
project, and others as well.
Read again: PRIMARY cause of project failure. I ought not to relive
that experience.

The same cautious tale applies to the contents
src/lib/libast/misc/state.c. You're having good intentions there. But
this is the way to a hell made out of context switches at the module
boundaries.

You'll find that opinion in line with the design of POSIX. Yes, they
have thread-local variables, but only optionally, and they are not
used in any POSIX API like openat() - it only has AT_FDCWD as global
cwd but no thread-local equivalent. Guess why?

The only exception - by accident and stupidity - has been uselocale()
- but even there was even a huge fracas when it was introduced, and
may now be depreciated again at the behalf of the NetBSD community
because thread-local variables aren't portable, or are only portable
if you accept that some platforms can implement thread-local variables
only via a table lookup (which makes it very, very slow).

> * although syscall restart on interrupt is part of posix, no 2 unix 
> implementations
>   apply restart in the same way on the same set of syscalls -- e.g., the 
> intesection
>   between the ast intercepts and any unix implementation != ast intercepts

Could you give details at this one? This may be a gap in either the
specification or conformance testing.

Irek
_______________________________________________
ast-developers mailing list
[email protected]
http://lists.research.att.com/mailman/listinfo/ast-developers

Reply via email to