Re: openat/fstatat functions implementation

[Antti Kantee <pooka%cs.hut.fi@localhost>]

On Sat Oct 03 2009 at 01:57:37 +0200, Adam Hamsik wrote:
> >>Do you have any comments ?
> 
> Here is last version of openat/fstatat diff I have written atf tests  
> for it and tested my patch by doing build(checks namei) and zfs(checks  
> openat/fstatat) + atf tests. Both cases were working. Can put these  
> syscalls in ?

A few comments:

* maybe i'm missing something obvious, but i don't see how openat()
  can work as specified since vn_open() does not do nameiat().  Also,
  your test doesn't seem to do a proper job testing, since it always
  opens under cwd.

+               /* XXX where is this released ? */
+               VREF(startvn);
* very discouraging comments.  I absolutely do not want any patches
  with author confidence like this.  It's hard enough to keep the
  kernel working even when people try to be diligent and prove to
  themselves that their code works (the proof might be wrong, and
  happens to me all the time, but that's another thing ;)

* un-clean patch, e.g.:
-       return (0);
+       return  (0);
+}
+int

* did you try running your atf test cases with atf?

* >10 lines duplicate copypasted code

(And I'll assume you got David's blessing for the namei part.)


Finally, on the ever-so-fascinating meta-discussion front, I think
these should be regular syscalls for now.  Maybe the variety people
are on to something, but conversion can happen later if some day there
is sensible infrastructure in place.  I'm especially worried about the
extra complexity Christos already mentioned.