On Thu, Mar 3, 2011 at 2:01 PM, M. Mohan Kumar <mo...@in.ibm.com> wrote: > On Thursday 03 March 2011 5:08:10 pm Stefan Hajnoczi wrote: >> On Mon, Feb 28, 2011 at 11:22 AM, M. Mohan Kumar <mo...@in.ibm.com> wrote: >> > + retval = recvmsg(sockfd, &msg, 0); >> > + if (retval < 0) { >> > + *sock_error = 1; >> > + return -EIO; >> > + } >> >> Are we guaranteed this will be called with signals blocked? Otherwise >> we need to handle EINTR. > > Ok > >> >> > + if (fd_info.fi_flags & FI_FD_SOCKERR) { >> > + *sock_error = 1; >> > + return -EIO; >> > + } >> > + /* If fd is invalid, ancillary data is not present */ >> > + if (fd_info.fi_fd < 0 || fd_info.fi_flags & FI_FD_INVALID) { >> > + return fd_info.fi_fd; >> > + } >> >> Testing fd_info.fi_flags & FI_FD_INVALID looks dangerous to me. If >> for some reason fi_fd >= 0 then we'd return success here. fd_fd < 0 >> should be a sufficient check, perhaps you wanted an assert() instead? > > This check is required because, > > Creating special objects like directory, device nodes will not have a valid > fd, usually fd will be 0 on success and -ve on error. During success cases, we > can't do sendmsg for fd=0 value with SCM_RIGHTS, that would result in problem. > In this case, we set fd_info.fi_flags to FI_FD_INVALID indicating fd is not a > valid one, but its not an error case also.
+ /* If fd is invalid, ancillary data is not present */ + if (fd_info.fi_fd < 0 || fd_info.fi_flags & FI_FD_INVALID) { if (fd_info.fi_fd < 0) { + return fd_info.fi_fd; + } We can get here now when no fd has been sent, but that's okay because... + + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) { + if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) || + cmsg->cmsg_level != SOL_SOCKET || + cmsg->cmsg_type != SCM_RIGHTS) { + continue; + } + fd = *((int *)CMSG_DATA(cmsg)); + return fd; + } + *sock_error = 1; + return -EIO; ...no SCM_RIGHTS was sent (it was optional) so instead of considering this case an error, we have reached a success case without an fd: return 0; Stefan