Our close-on-exec flag handling was broken: it's supposed to be specified per slot, not

per file descriptor (a descriptor can be shared among several slots). There is now a
second table in the io_context structure that contains that information in a bitmap.
There are now two new (private) functions to control the close-on-exec flag, fd_close_on_exec(),
and fd_set_close_on_exec().
F_DUPFD, dup(), and dup2() are supposed to clear the close-on-exec flag on the duplicated
slot - this fixes bug #57 (no output after a redirect of a shell builtin).


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@14313 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/private/kernel/fs/fd.h

@@ -74,6 +74,9 @@ extern status_t select_fd(int fd, uint8 event, uint32 ref, struct select_sync *s
 extern status_t deselect_fd(int fd, uint8 event, struct select_sync *sync, bool kernel);
 extern bool fd_is_valid(int fd, bool kernel);
 
+extern bool fd_close_on_exec(struct io_context *context, int fd);
+extern void fd_set_close_on_exec(struct io_context *context, int fd, bool closeFD);
+
 static struct io_context *get_current_io_context(bool kernel);
 
 /* The prototypes of the (sys|user)_ functions are currently defined in vfs.h */

headers/private/kernel/vfs.h

@@ -37,6 +37,7 @@ typedef struct io_context {
 	uint32		table_size;
 	uint32		num_used_fds;
 	struct file_descriptor **fds;
+	uint8		*fds_close_on_exec;
 	struct list node_monitors;
 	uint32		num_monitors;
 	uint32		max_monitors;

src/system/kernel/fs/fd.c

@@ -61,6 +61,23 @@ alloc_fd(void)
 }
 
 
+bool
+fd_close_on_exec(struct io_context *context, int fd)
+{
+	return CHECK_BIT(context->fds_close_on_exec[fd / 8], fd & 7) ? true : false;
+}
+
+
+void
+fd_set_close_on_exec(struct io_context *context, int fd, bool closeFD)
+{
+	if (closeFD)
+		context->fds_close_on_exec[fd / 8] |= (1 << (fd & 7));
+	else
+		context->fds_close_on_exec[fd / 8] &= ~(1 << (fd & 7));
+}
+
+
 /** Searches a free slot in the FD table of the provided I/O context, and inserts
  *	the specified descriptor into it.
  */
@@ -176,6 +193,7 @@ remove_fd(struct io_context *context, int fd)
 
 	if (descriptor)	{	// fd is valid
 		context->fds[fd] = NULL;
+		fd_set_close_on_exec(context, fd, false);
 		context->num_used_fds--;
 	}
 
@@ -203,6 +221,11 @@ dup_fd(int fd, bool kernel)
 	status = new_fd(context, descriptor);
 	if (status < 0)
 		put_fd(descriptor);
+	else {
+		mutex_lock(&context->io_mutex);
+		fd_set_close_on_exec(context, status, false);
+		mutex_unlock(&context->io_mutex);
+	}
 
 	return status;
 }
@@ -254,6 +277,8 @@ dup2_fd(int oldfd, int newfd, bool kernel)
 			context->num_used_fds++;
 	}
 
+	fd_set_close_on_exec(context, newfd, false);
+
 	mutex_unlock(&context->io_mutex);
 
 	// Say bye bye to the evicted fd

src/system/kernel/fs/vfs.cpp

@@ -2826,14 +2826,13 @@ vfs_exec_io_context(void *_context)
 	struct io_context *context = (struct io_context *)_context;
 	uint32 i;
 
-
 	for (i = 0; i < context->table_size; i++) {
 		mutex_lock(&context->io_mutex);
 
 		struct file_descriptor *descriptor = context->fds[i];
 		bool remove = false;
 
-		if (descriptor != NULL && (descriptor->open_mode & O_CLOEXEC) != 0) {
+		if (descriptor != NULL && fd_close_on_exec(context, i)) {
 			context->fds[i] = NULL;
 			context->num_used_fds--;
 
@@ -2873,13 +2872,17 @@ vfs_new_io_context(void *_parentContext)
 	else
 		tableSize = DEFAULT_FD_TABLE_SIZE;
 
-	context->fds = (file_descriptor **)malloc(sizeof(struct file_descriptor *) * tableSize);
+	// allocate space for FDs and their close-on-exec flag
+	context->fds = (file_descriptor **)malloc(sizeof(struct file_descriptor *) * tableSize
+		+ tableSize / 8);
 	if (context->fds == NULL) {
 		free(context);
 		return NULL;
 	}
 
-	memset(context->fds, 0, sizeof(struct file_descriptor *) * tableSize);
+	memset(context->fds, 0, sizeof(struct file_descriptor *) * tableSize
+		+ tableSize / 8);
+	context->fds_close_on_exec = (uint8 *)(context->fds + tableSize);
 
 	if (mutex_init(&context->io_mutex, "I/O context") < 0) {
 		free(context->fds);
@@ -2901,7 +2904,7 @@ vfs_new_io_context(void *_parentContext)
 		for (i = 0; i < tableSize; i++) {
 			struct file_descriptor *descriptor = parentContext->fds[i];
 
-			if (descriptor != NULL && (descriptor->open_mode & O_CLOEXEC) == 0) {
+			if (descriptor != NULL && !fd_close_on_exec(parentContext, i)) {
 				context->fds[i] = descriptor;
 				context->num_used_fds++;
 				atomic_add(&descriptor->ref_count, 1);
@@ -3840,21 +3843,29 @@ common_fcntl(int fd, int op, uint32 argument, bool kernel)
 
 	switch (op) {
 		case F_SETFD:
+		{
+			struct io_context *context = get_current_io_context(kernel);
 			// Set file descriptor flags
 
 			// O_CLOEXEC is the only flag available at this time
-			if (argument == FD_CLOEXEC)
+			mutex_lock(&context->io_mutex);
-				atomic_or(&descriptor->open_mode, O_CLOEXEC);
+			fd_set_close_on_exec(context, fd, argument == FD_CLOEXEC);
-			else
+			mutex_unlock(&context->io_mutex);
-				atomic_and(&descriptor->open_mode, O_CLOEXEC);
 			
 			status = B_OK;
 			break;
+		}
 
 		case F_GETFD:
+		{
+			struct io_context *context = get_current_io_context(kernel);
+
 			// Get file descriptor flags
-			status = (descriptor->open_mode & O_CLOEXEC) ? FD_CLOEXEC : 0;
+			mutex_lock(&context->io_mutex);
+			status = fd_close_on_exec(context, fd) ? FD_CLOEXEC : 0;
+			mutex_unlock(&context->io_mutex);
 			break;
+		}
 
 		case F_SETFL:
 			// Set file descriptor open mode
@@ -3877,10 +3888,19 @@ common_fcntl(int fd, int op, uint32 argument, bool kernel)
 			break;
 
 		case F_DUPFD:
-			status = new_fd_etc(get_current_io_context(kernel), descriptor, (int)argument);
+		{
-			if (status >= 0)
+			struct io_context *context = get_current_io_context(kernel);
+
+			status = new_fd_etc(context, descriptor, (int)argument);
+			if (status >= 0) {
+				mutex_lock(&context->io_mutex);
+				fd_set_close_on_exec(context, fd, false);
+				mutex_unlock(&context->io_mutex);
+
 				atomic_add(&descriptor->ref_count, 1);
+			}
 			break;
+		}
 
 		case F_GETLK:
 			status = get_advisory_lock(descriptor->u.vnode, &flock);