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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
