review request: making openat(2) a cancellation point

[Jilles Tjoelker]

While implementing a faster, cleaner and more compliant getcwd(3) using
*at functions, I noticed that openat() was not a cancellation point
(unlike open()). I think that should be fixed first, both for its own
sake and because getcwd() will need to use _openat.

I created the attached patch. It seems to work, I can still compile and
run non-threaded and threaded programs using openat() and I can cancel a
thread blocked in openat() (such as trying to open a fifo). I can also
use _openat in libc. However, I think some review is appropriate because
it all looks fairly complicated.

Because libkse and libc_r don't seem to be built, I have not updated
them.

Note that this will cause threaded 9.x binaries that use openat() to
break on 8.x. This is to be expected.

--
Jilles Tjoelker

---

Index: lib/libc/include/namespace.h
===================================================================
--- lib/libc/include/namespace.h (revision 197481)
+++ lib/libc/include/namespace.h (working copy)
@@ -80,6 +80,7 @@
 #define listen _listen
 #define nanosleep _nanosleep
 #define open _open
+#define openat _openat
 #define poll _poll
 #define pthread_atfork _pthread_atfork
 #define pthread_attr_destroy _pthread_attr_destroy
Index: lib/libc/include/un-namespace.h
===================================================================
--- lib/libc/include/un-namespace.h (revision 197481)
+++ lib/libc/include/un-namespace.h (working copy)
@@ -61,6 +61,7 @@
 #undef listen
 #undef nanosleep
 #undef open
+#undef openat
 #undef poll
 #undef pthread_atfork
 #undef pthread_attr_destroy
Index: lib/libc/sys/Symbol.map
===================================================================
--- lib/libc/sys/Symbol.map (revision 197481)
+++ lib/libc/sys/Symbol.map (working copy)
@@ -769,6 +769,8 @@ FBSDprivate_1.0 {
  __sys_olio_listio;
  _open;
  __sys_open;
+ _openat;
+ __sys_openat;
  _pathconf;
  __sys_pathconf;
  _pipe;
Index: lib/libthr/pthread.map
===================================================================
--- lib/libthr/pthread.map (revision 197481)
+++ lib/libthr/pthread.map (working copy)
@@ -195,6 +195,7 @@ FBSDprivate_1.0 {
  __msync;
  __nanosleep;
  __open;
+ __openat;
  __poll;
  __pthread_cond_timedwait;
  __pthread_cond_wait;
@@ -406,3 +407,7 @@ FBSD_1.1 {
  pthread_mutex_setspinloops_np;
  pthread_mutex_setyieldloops_np;
 };
+
+FBSD_1.2 {
+ openat;
+};
Index: lib/libthr/thread/thr_syscalls.c
===================================================================
--- lib/libthr/thread/thr_syscalls.c (revision 197481)
+++ lib/libthr/thread/thr_syscalls.c (working copy)
@@ -139,6 +139,7 @@ int __fsync(int);
 int __msync(void *, size_t, int);
 int __nanosleep(const struct timespec *, struct timespec *);
 int __open(const char *, int,...);
+int __openat(int, const char *, int,...);
 int __poll(struct pollfd *, unsigned int, int);
 ssize_t __read(int, void *buf, size_t);
 ssize_t __readv(int, const struct iovec *, int);
@@ -341,6 +342,33 @@ __open(const char *path, int flags,...)
  return ret;
 }
 
+__weak_reference(__openat, openat);
+
+int
+__openat(int fd, const char *path, int flags,...)
+{
+ struct pthread *curthread = _get_curthread();
+ int ret;
+ int mode = 0;
+ va_list ap;
+
+ _thr_cancel_enter(curthread);
+
+ /* Check if the file is being created: */
+ if (flags & O_CREAT) {
+ /* Get the creation mode: */
+ va_start(ap, flags);
+ mode = va_arg(ap, int);
+ va_end(ap);
+ }
+
+ ret = __sys_openat(fd, path, flags, mode);
+
+ _thr_cancel_leave(curthread);
+
+ return ret;
+}
+
 __weak_reference(__poll, poll);
 
 int
Index: lib/libthr/thread/thr_private.h
===================================================================
--- lib/libthr/thread/thr_private.h (revision 197481)
+++ lib/libthr/thread/thr_private.h (working copy)
@@ -668,6 +668,7 @@ void _pthread_cleanup_pop(int);
 #ifdef  _SYS_FCNTL_H_
 int     __sys_fcntl(int, int, ...);
 int     __sys_open(const char *, int, ...);
+int     __sys_openat(int, const char *, int, ...);
 #endif
 
 /* #include <signal.h> */

---

_______________________________________________
freebsd-threads@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-threads
To unsubscribe, send any mail to "freebsd-threads-unsubscribe@..."

[Daniel Eischen]

On Mon, 5 Oct 2009, Jilles Tjoelker wrote:

> While implementing a faster, cleaner and more compliant getcwd(3) using
> *at functions, I noticed that openat() was not a cancellation point
> (unlike open()). I think that should be fixed first, both for its own
> sake and because getcwd() will need to use _openat.

POSIX states that openat() is a cancellation point, so
it should be changed accordingly.  I have not reviewed
th patch, but the intent is valid :-)

> I created the attached patch. It seems to work, I can still compile and
> run non-threaded and threaded programs using openat() and I can cancel a
> thread blocked in openat() (such as trying to open a fifo). I can also
> use _openat in libc. However, I think some review is appropriate because
> it all looks fairly complicated.
>
> Because libkse and libc_r don't seem to be built, I have not updated
> them.
>
> Note that this will cause threaded 9.x binaries that use openat() to
> break on 8.x. This is to be expected.

--
DE
_______________________________________________
freebsd-threads@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-threads
To unsubscribe, send any mail to "freebsd-threads-unsubscribe@..."

[John Baldwin]

On Monday 05 October 2009 1:17:29 pm Jilles Tjoelker wrote:

> While implementing a faster, cleaner and more compliant getcwd(3) using
> *at functions, I noticed that openat() was not a cancellation point
> (unlike open()). I think that should be fixed first, both for its own
> sake and because getcwd() will need to use _openat.
>
> I created the attached patch. It seems to work, I can still compile and
> run non-threaded and threaded programs using openat() and I can cancel a
> thread blocked in openat() (such as trying to open a fifo). I can also
> use _openat in libc. However, I think some review is appropriate because
> it all looks fairly complicated.
>
> Because libkse and libc_r don't seem to be built, I have not updated
> them.
>
> Note that this will cause threaded 9.x binaries that use openat() to
> break on 8.x. This is to be expected.

From what I can tell the patch looks fine to me.

--
John Baldwin
_______________________________________________
freebsd-threads@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-threads
To unsubscribe, send any mail to "freebsd-threads-unsubscribe@..."