[Spice-devel] [PATCH 03/17] Move sync_write* to reds_stream.h
Alon Levy
alevy at redhat.com
Sun Jan 12 03:04:10 PST 2014
On 01/07/2014 01:14 PM, Christophe Fergeau wrote:
> They are renamed to reds_stream_write*
You also changed the return type for reds_stream_write_all (was
sync_write) to bool, which is now being cast to int in some code paths.
Not an objection, but perhaps worth a later patch (unless it's already
addressed, I'm commenting while reading so haven't read the later
patches yet).
> ---
> server/reds.c | 78 ++++++++++++++++------------------------------------
> server/reds_stream.c | 28 +++++++++++++++++++
> server/reds_stream.h | 5 ++++
> 3 files changed, 57 insertions(+), 54 deletions(-)
>
> diff --git a/server/reds.c b/server/reds.c
> index 81e90f3..ce8bb97 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -1395,24 +1395,6 @@ int reds_handle_migrate_data(MainChannelClient *mcc, SpiceMigrateDataMain *mig_d
> return TRUE;
> }
>
> -static int sync_write(RedsStream *stream, const void *in_buf, size_t n)
> -{
> - const uint8_t *buf = (uint8_t *)in_buf;
> -
> - while (n) {
> - int now = reds_stream_write(stream, buf, n);
> - if (now <= 0) {
> - if (now == -1 && (errno == EINTR || errno == EAGAIN)) {
> - continue;
> - }
> - return FALSE;
> - }
> - n -= now;
> - buf += now;
> - }
> - return TRUE;
> -}
> -
> static void reds_channel_init_auth_caps(RedLinkInfo *link, RedChannel *channel)
> {
> if (sasl_enabled && !link->skip_auth) {
> @@ -1473,13 +1455,13 @@ static int reds_send_link_ack(RedLinkInfo *link)
> BIO_get_mem_ptr(bio, &bmBuf);
> memcpy(ack.pub_key, bmBuf->data, sizeof(ack.pub_key));
>
> - if (!sync_write(link->stream, &header, sizeof(header)))
> + if (!reds_stream_write_all(link->stream, &header, sizeof(header)))
> goto end;
> - if (!sync_write(link->stream, &ack, sizeof(ack)))
> + if (!reds_stream_write_all(link->stream, &ack, sizeof(ack)))
> goto end;
> - if (!sync_write(link->stream, channel_caps->common_caps, channel_caps->num_common_caps * sizeof(uint32_t)))
> + if (!reds_stream_write_all(link->stream, channel_caps->common_caps, channel_caps->num_common_caps * sizeof(uint32_t)))
> goto end;
> - if (!sync_write(link->stream, channel_caps->caps, channel_caps->num_caps * sizeof(uint32_t)))
> + if (!reds_stream_write_all(link->stream, channel_caps->caps, channel_caps->num_caps * sizeof(uint32_t)))
> goto end;
>
> ret = TRUE;
> @@ -1500,7 +1482,7 @@ static int reds_send_link_error(RedLinkInfo *link, uint32_t error)
> header.minor_version = SPICE_VERSION_MINOR;
> memset(&reply, 0, sizeof(reply));
> reply.error = error;
> - return sync_write(link->stream, &header, sizeof(header)) && sync_write(link->stream, &reply,
> + return reds_stream_write_all(link->stream, &header, sizeof(header)) && reds_stream_write_all(link->stream, &reply,
> sizeof(reply));
> }
>
> @@ -1522,7 +1504,7 @@ static void reds_info_new_channel(RedLinkInfo *link, int connection_id)
>
> static void reds_send_link_result(RedLinkInfo *link, uint32_t error)
> {
> - sync_write(link->stream, &error, sizeof(error));
> + reds_stream_write_all(link->stream, &error, sizeof(error));
> }
>
> int reds_expects_link_id(uint32_t connection_id)
> @@ -1977,18 +1959,6 @@ static inline void async_read_clear_handlers(AsyncRead *obj)
> reds_stream_remove_watch(obj->stream);
> }
>
> -#if HAVE_SASL
> -static int sync_write_u8(RedsStream *s, uint8_t n)
> -{
> - return sync_write(s, &n, sizeof(uint8_t));
> -}
> -
> -static int sync_write_u32(RedsStream *s, uint32_t n)
> -{
> - return sync_write(s, &n, sizeof(uint32_t));
> -}
> -#endif
> -
> static void async_read_handler(int fd, int event, void *data)
> {
> AsyncRead *obj = (AsyncRead *)data;
> @@ -2157,14 +2127,14 @@ static void reds_handle_auth_sasl_step(void *opaque)
>
> if (serveroutlen) {
> serveroutlen += 1;
> - sync_write(link->stream, &serveroutlen, sizeof(uint32_t));
> - sync_write(link->stream, serverout, serveroutlen);
> + reds_stream_write_all(link->stream, &serveroutlen, sizeof(uint32_t));
> + reds_stream_write_all(link->stream, serverout, serveroutlen);
> } else {
> - sync_write(link->stream, &serveroutlen, sizeof(uint32_t));
> + reds_stream_write_all(link->stream, &serveroutlen, sizeof(uint32_t));
> }
>
> /* Whether auth is complete */
> - sync_write_u8(link->stream, err == SASL_CONTINUE ? 0 : 1);
> + reds_stream_write_u8(link->stream, err == SASL_CONTINUE ? 0 : 1);
>
> if (err == SASL_CONTINUE) {
> spice_info("%s", "Authentication must continue (step)");
> @@ -2182,7 +2152,7 @@ static void reds_handle_auth_sasl_step(void *opaque)
> }
>
> spice_info("Authentication successful");
> - sync_write_u32(link->stream, SPICE_LINK_ERR_OK); /* Accept auth */
> + reds_stream_write_u32(link->stream, SPICE_LINK_ERR_OK); /* Accept auth */
>
> /*
> * Delay writing in SSF encoded until now
> @@ -2196,9 +2166,9 @@ static void reds_handle_auth_sasl_step(void *opaque)
> return;
>
> authreject:
> - sync_write_u32(link->stream, 1); /* Reject auth */
> - sync_write_u32(link->stream, sizeof("Authentication failed"));
> - sync_write(link->stream, "Authentication failed", sizeof("Authentication failed"));
> + reds_stream_write_u32(link->stream, 1); /* Reject auth */
> + reds_stream_write_u32(link->stream, sizeof("Authentication failed"));
> + reds_stream_write_all(link->stream, "Authentication failed", sizeof("Authentication failed"));
>
> authabort:
> reds_link_free(link);
> @@ -2288,14 +2258,14 @@ static void reds_handle_auth_sasl_start(void *opaque)
>
> if (serveroutlen) {
> serveroutlen += 1;
> - sync_write(link->stream, &serveroutlen, sizeof(uint32_t));
> - sync_write(link->stream, serverout, serveroutlen);
> + reds_stream_write_all(link->stream, &serveroutlen, sizeof(uint32_t));
> + reds_stream_write_all(link->stream, serverout, serveroutlen);
> } else {
> - sync_write(link->stream, &serveroutlen, sizeof(uint32_t));
> + reds_stream_write_all(link->stream, &serveroutlen, sizeof(uint32_t));
> }
>
> /* Whether auth is complete */
> - sync_write_u8(link->stream, err == SASL_CONTINUE ? 0 : 1);
> + reds_stream_write_u8(link->stream, err == SASL_CONTINUE ? 0 : 1);
>
> if (err == SASL_CONTINUE) {
> spice_info("%s", "Authentication must continue (start)");
> @@ -2313,7 +2283,7 @@ static void reds_handle_auth_sasl_start(void *opaque)
> }
>
> spice_info("Authentication successful");
> - sync_write_u32(link->stream, SPICE_LINK_ERR_OK); /* Accept auth */
> + reds_stream_write_u32(link->stream, SPICE_LINK_ERR_OK); /* Accept auth */
>
> /*
> * Delay writing in SSF encoded until now
> @@ -2327,9 +2297,9 @@ static void reds_handle_auth_sasl_start(void *opaque)
> return;
>
> authreject:
> - sync_write_u32(link->stream, 1); /* Reject auth */
> - sync_write_u32(link->stream, sizeof("Authentication failed"));
> - sync_write(link->stream, "Authentication failed", sizeof("Authentication failed"));
> + reds_stream_write_u32(link->stream, 1); /* Reject auth */
> + reds_stream_write_u32(link->stream, sizeof("Authentication failed"));
> + reds_stream_write_all(link->stream, "Authentication failed", sizeof("Authentication failed"));
>
> authabort:
> reds_link_free(link);
> @@ -2529,8 +2499,8 @@ static void reds_start_auth_sasl(RedLinkInfo *link)
> sasl->mechlist = spice_strdup(mechlist);
>
> mechlistlen = strlen(mechlist);
> - if (!sync_write(link->stream, &mechlistlen, sizeof(uint32_t))
> - || !sync_write(link->stream, sasl->mechlist, mechlistlen)) {
> + if (!reds_stream_write_all(link->stream, &mechlistlen, sizeof(uint32_t))
> + || !reds_stream_write_all(link->stream, sasl->mechlist, mechlistlen)) {
> spice_warning("SASL mechanisms write error");
> goto error;
> }
> diff --git a/server/reds_stream.c b/server/reds_stream.c
> index 7adc745..093621f 100644
> --- a/server/reds_stream.c
> +++ b/server/reds_stream.c
> @@ -57,6 +57,24 @@ ssize_t reds_stream_read(RedsStream *s, void *buf, size_t nbyte)
> return ret;
> }
>
> +bool reds_stream_write_all(RedsStream *stream, const void *in_buf, size_t n)
> +{
> + const uint8_t *buf = (uint8_t *)in_buf;
> +
> + while (n) {
> + int now = reds_stream_write(stream, buf, n);
> + if (now <= 0) {
> + if (now == -1 && (errno == EINTR || errno == EAGAIN)) {
> + continue;
> + }
> + return FALSE;
> + }
> + n -= now;
> + buf += now;
> + }
> + return TRUE;
> +}
> +
> static ssize_t reds_stream_sasl_write(RedsStream *s, const void *buf, size_t nbyte);
>
> ssize_t reds_stream_write(RedsStream *s, const void *buf, size_t nbyte)
> @@ -132,6 +150,16 @@ void reds_stream_push_channel_event(RedsStream *s, int event)
> }
>
> #if HAVE_SASL
> +bool reds_stream_write_u8(RedsStream *s, uint8_t n)
> +{
> + return reds_stream_write_all(s, &n, sizeof(uint8_t));
> +}
> +
> +bool reds_stream_write_u32(RedsStream *s, uint32_t n)
> +{
> + return reds_stream_write_all(s, &n, sizeof(uint32_t));
> +}
> +
> static ssize_t reds_stream_sasl_write(RedsStream *s, const void *buf, size_t nbyte)
> {
> ssize_t ret;
> diff --git a/server/reds_stream.h b/server/reds_stream.h
> index ca18f75..c725414 100644
> --- a/server/reds_stream.h
> +++ b/server/reds_stream.h
> @@ -21,6 +21,8 @@
> #include "spice.h"
> #include "common/mem.h"
>
> +#include <stdbool.h>
> +
> #include <openssl/ssl.h>
>
> #if HAVE_SASL
> @@ -92,6 +94,9 @@ typedef enum {
> ssize_t reds_stream_read(RedsStream *s, void *buf, size_t nbyte);
> ssize_t reds_stream_write(RedsStream *s, const void *buf, size_t nbyte);
> ssize_t reds_stream_writev(RedsStream *s, const struct iovec *iov, int iovcnt);
> +bool reds_stream_write_all(RedsStream *stream, const void *in_buf, size_t n);
> +bool reds_stream_write_u8(RedsStream *s, uint8_t n);
> +bool reds_stream_write_u32(RedsStream *s, uint32_t n);
> void reds_stream_free(RedsStream *s);
>
> void reds_stream_push_channel_event(RedsStream *s, int event);
>
More information about the Spice-devel
mailing list