On Sat, Oct 19, 2013 at 04:39:13PM +0200, Dominik Paulus wrote: > @@ -104,8 +105,10 @@ static int import_device(int sockfd, struct usbip_usb_device *udev) > return -1; > } > > - rc = usbip_vhci_attach_device(port, sockfd, udev->busnum, > + usbip_net_bye(conn); > + rc = usbip_vhci_attach_device(port, conn->sockfd, udev->busnum, > udev->devnum, udev->speed); > + > if (rc < 0) { Don't put a blank line between the function call and the check. They logically are one idea. > > - rc = usbip_net_recv(sockfd, (void *) &reply, sizeof(reply)); > + rc = usbip_net_recv(conn, (void *) &reply, sizeof(reply)); There is no need to cast to void here, btw. That's just noise. > do { > - if (sending) > - nbytes = send(sockfd, buff, bufflen, 0); > + if (!conn->have_crypto && sending) > + nbytes = send(conn->sockfd, buff, bufflen, 0); > + else if (!conn->have_crypto && !sending) > + nbytes = recv(conn->sockfd, buff, bufflen, MSG_WAITALL); > +#ifdef HAVE_GNUTLS > + else if (sending) > + nbytes = gnutls_record_send(conn->session, buff, bufflen); > else > - nbytes = recv(sockfd, buff, bufflen, MSG_WAITALL); > + nbytes = gnutls_record_recv(conn->session, buff, bufflen); > +#else > + /* > + * Assertion to let gcc be able to infer proper initialization > + * of nbytes. > + */ > + assert(!conn->have_crypto); > +#endif This is messy and I feel like it should be abstracted into a function so we can hide the ifdef in a header file. if (sending) nbytes = usbip_send(conn, buff, bufflen, 0); else nbytes = usbip_recv(... We'd still have the ifdef but hidden away. > +int usbip_net_srp_server_handshake(struct usbip_connection *conn) > +{ > + int ret; > + > + if (gnutls_init(&conn->session, GNUTLS_SERVER) != 0) > + return -1; > + gnutls_priority_set_direct(conn->session, "NORMAL:-KX-ALL:+SRP", NULL); > + if (gnutls_credentials_set(conn->session, GNUTLS_CRD_SRP, > + usbip_net_srp_cred) != 0) > + return -1; > + Kernel style is more beautiful: ret = gnutls_credentials_set(conn->session, GNUTLS_CRD_SRP, usbip_net_srp_cred); if (ret) return ret; > +void usbip_net_bye(struct usbip_connection *conn) > +{ > +#ifdef HAVE_GNUTLS > + if (conn->have_crypto) { > + gnutls_bye(conn->session, GNUTLS_SHUT_RDWR); > + > + gnutls_deinit(conn->session); > + if (!conn->server) > + gnutls_srp_free_client_credentials(conn->srp_client_cred); > + > + conn->have_crypto = 0; > + } > +#else > + (void)conn; What is this about? I assume that GCC warns, but which version of GCC are you using because that sounds horrible. > +#endif regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel