On Wed, Jun 13, 2012 at 01:06:56PM -0700, Ethan Jackson wrote:
> > + retval = recvmsg(sock, &msg, 0);
> > + } while (retval < 0 && errno == EINTR);
> > + if (retval <= 0) {
> > + return retval < 0 ? -errno : 0;
> > + }
> > +
> > + for (p = CMSG_FIRSTHDR(&msg); p; p = CMSG_NXTHDR(&msg, p)) {
> > + if (p->cmsg_level != SOL_SOCKET || p->cmsg_type != SCM_RIGHTS) {
> > + VLOG_ERR("unexpected control message %d:%d",
> > + (int) p->cmsg_level, (int) p->cmsg_type);
>
> Are the casts necessary here? At least in the sparse version of the
> header the type is int.
POSIX says they're "int"s. Thanks, I removed the casts.
> > + goto error;
> > + } else if (*n_fdsp) {
> > + VLOG_ERR("multiple SCM_RIGHTS received");
> > + goto error;
> > + } else {
> > + size_t n_fds = (p->cmsg_len - CMSG_LEN(0)) / sizeof *fds;
> > + const int *fds_data = (const int *) CMSG_DATA(p);
> > +
> > + assert(n_fds > 0);
> > + if (n_fds > SOUTIL_MAX_FDS) {
> > + VLOG_ERR("%zu fds received but only %d supported",
> > + n_fds, SOUTIL_MAX_FDS);
> > + for (i = 0; i < n_fds; i++) {
> > + close(fds_data[i]);
> > + }
> > + goto error;
> > + }
> > +
> > + *n_fdsp = n_fds;
> > + memcpy(fds, fds_data, n_fds * sizeof *fds);
> > + }
> > + }
> > +
> > + return retval;
> > +
> > +error:
>
> > + for (i = 0; i < *n_fdsp; i++) {
> > + close(fds[i]);
> > + }
>
> This doesn't seem right to me. fds is only written to when there's no
> error. Really this should be closing the file descriptors in
> fds_data, but that's done already. If it turns out this is
> unnecessary we can probably remove the goto altogether and just use
> return EPROTO.
It's a loop, so there could have been fds from earlier iterations of the
loop. Right?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
