Re: [PATCHv2 4/4] first use of sys_indirect system call
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 dean gaudet wrote: > i'm not suggesting the library set the global flag. i'm suggesting that > me as an app writer will do so. > > it seems like both methods are useful. No, the global flag is hardly ever useful. You almost never know the details of all the libraries you link to well enough to determine that they don't need FD_CLOEXEC disabled. Even more problematic, you cannot know whether they will need it in future. For applications the solution is simple: wrap to appropriate calls. Apache has all these apr_ wrappers. But them to some good news after all. - -- ➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iD8DBQFHPeim2ijCOnn/RHQRAu8xAJsF/0Ir1PWMbHkVRaI5vKOGFS4tMACfVEs9 pMYAiCAU1E2B+7QR0EP+/F8= =btt9 -END PGP SIGNATURE- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/4] first use of sys_indirect system call
On Fri, 16 Nov 2007, Ulrich Drepper wrote: > dean gaudet wrote: > > honestly i think there should be a per-task flag which indicates whether > > fds are by default F_CLOEXEC or not. my reason: third party libraries. > > Only somebody who thinks exclusively about applications as opposed to > runtimes/libraries can say something like that. Library writers don't > have the luxury of being able to modify any global state. This has all > been discussed here before. only someone who thinks about writing libraries can say something like that. you've solved the problem for yourself, and for well written applications, but not for the other 99.% of libraries out there. i'm not suggesting the library set the global flag. i'm suggesting that me as an app writer will do so. it seems like both methods are useful. -dean - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/4] first use of sys_indirect system call
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 dean gaudet wrote: > honestly i think there should be a per-task flag which indicates whether > fds are by default F_CLOEXEC or not. my reason: third party libraries. Only somebody who thinks exclusively about applications as opposed to runtimes/libraries can say something like that. Library writers don't have the luxury of being able to modify any global state. This has all been discussed here before. - -- ➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) iD8DBQFHPd8b2ijCOnn/RHQRAuPPAKCm5mcOl8dycDenxi7BNFdrf2IfWgCgmaXQ Fj7V13HU1vX6fM9bRumxRpk= =UIi1 -END PGP SIGNATURE- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/4] first use of sys_indirect system call
you know... i understand the need for FD_CLOEXEC -- in fact i tried petitioning for CLOEXEC options to all the fd creating syscalls something like 7 years ago when i was banging my head against the wall trying to figure out how to thread apache... but even still i'm not convinced that extending every system call which creates an fd is the way to do this. honestly i think there should be a per-task flag which indicates whether fds are by default F_CLOEXEC or not. my reason: third party libraries. i can control all my own code in a threaded program, but i can't control all the code which is linked in. fds are going to leak. if i set a per task flag then the only thing which would break are third party libraries which use fork/exec and aren't aware they may need to unset F_CLOEXEC. personally i'd rather break that than leak fds to another program. but hey i'm happy to see this sort of thing is finally being fixed, thanks. -dean - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 4/4] first use of sys_indirect system call
Ulrich Drepper a écrit : This is a first user of sys_indirect. Several of the socket-related system calls which produce a file handle now can be passed an additional parameter to set the FD_CLOEXEC flag. - retval = sock_map_fd(sock); + retval = sock_map_fd_flags(sock, current->indirect_params.file_flags.flags); Yes, we know now why you wanted sys_indirect so much :) However, there should be a macro or something to ease reader mind... #define INDPARAM(SUBSYSNAME,PARAMNAME) \ (current->indirect_params.##SUBSYSNAME.##PARAMNAME) > + retval = sock_map_fd_flags(sock, INDPARAM(file_flags,flags)); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv2 4/4] first use of sys_indirect system call
This is a first user of sys_indirect. Several of the socket-related system calls which produce a file handle now can be passed an additional parameter to set the FD_CLOEXEC flag. socket.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) --- a/net/socket.c +++ b/net/socket.c @@ -344,11 +344,11 @@ static struct dentry_operations sockfs_dentry_operations = { * but we take care of internal coherence yet. */ -static int sock_alloc_fd(struct file **filep) +static int sock_alloc_fd(struct file **filep, int flags) { int fd; - fd = get_unused_fd(); + fd = get_unused_fd_flags(flags); if (likely(fd >= 0)) { struct file *file = get_empty_filp(); @@ -391,10 +391,10 @@ static int sock_attach_fd(struct socket *sock, struct file *file) return 0; } -int sock_map_fd(struct socket *sock) +static int sock_map_fd_flags(struct socket *sock, int flags) { struct file *newfile; - int fd = sock_alloc_fd(&newfile); + int fd = sock_alloc_fd(&newfile, flags); if (likely(fd >= 0)) { int err = sock_attach_fd(sock, newfile); @@ -409,6 +409,11 @@ int sock_map_fd(struct socket *sock) return fd; } +int sock_map_fd(struct socket *sock) +{ + return sock_map_fd_flags(sock, 0); +} + static struct socket *sock_from_file(struct file *file, int *err) { if (file->f_op == &socket_file_ops) @@ -1208,7 +1213,7 @@ asmlinkage long sys_socket(int family, int type, int protocol) if (retval < 0) goto out; - retval = sock_map_fd(sock); + retval = sock_map_fd_flags(sock, current->indirect_params.file_flags.flags); if (retval < 0) goto out_release; @@ -1249,13 +1254,13 @@ asmlinkage long sys_socketpair(int family, int type, int protocol, if (err < 0) goto out_release_both; - fd1 = sock_alloc_fd(&newfile1); + fd1 = sock_alloc_fd(&newfile1, current->indirect_params.file_flags.flags); if (unlikely(fd1 < 0)) { err = fd1; goto out_release_both; } - fd2 = sock_alloc_fd(&newfile2); + fd2 = sock_alloc_fd(&newfile2, current->indirect_params.file_flags.flags); if (unlikely(fd2 < 0)) { err = fd2; put_filp(newfile1); @@ -1411,7 +1416,7 @@ asmlinkage long sys_accept(int fd, struct sockaddr __user *upeer_sockaddr, */ __module_get(newsock->ops->owner); - newfd = sock_alloc_fd(&newfile); + newfd = sock_alloc_fd(&newfile, current->indirect_params.file_flags.flags); if (unlikely(newfd < 0)) { err = newfd; sock_release(newsock); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/