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); > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel