All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Gerd Hoffmann" <kraxel@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"P J P" <ppandit@redhat.com>,
	"Daniel P. Berrange" <berrange@redhat.com>
Subject: [Qemu-devel] [PATCH v1 13/13] ui: mix misleading comments & return types of VNC I/O helper methods
Date: Mon, 18 Dec 2017 19:12:28 +0000	[thread overview]
Message-ID: <20171218191228.31018-14-berrange@redhat.com> (raw)
In-Reply-To: <20171218191228.31018-1-berrange@redhat.com>

While the QIOChannel APIs for reading/writing data return ssize_t, with negative
value indicating an error, the VNC code passes this return value through the
vnc_client_io_error() method. This detects the error condition, disconnects the
client and returns 0 to indicate error. Thus all the VNC helper methods should
return size_t (unsigned), and misleading comments which refer to the possibility
of negative return values need fixing.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 ui/vnc-auth-sasl.c |  8 ++++----
 ui/vnc-auth-sasl.h |  4 ++--
 ui/vnc.c           | 29 +++++++++++++++--------------
 ui/vnc.h           |  6 +++---
 4 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index 8c1cdde3db..74a5f513f2 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -48,9 +48,9 @@ void vnc_sasl_client_cleanup(VncState *vs)
 }
 
 
-long vnc_client_write_sasl(VncState *vs)
+size_t vnc_client_write_sasl(VncState *vs)
 {
-    long ret;
+    size_t ret;
 
     VNC_DEBUG("Write SASL: Pending output %p size %zd offset %zd "
               "Encoded: %p size %d offset %d\n",
@@ -106,9 +106,9 @@ long vnc_client_write_sasl(VncState *vs)
 }
 
 
-long vnc_client_read_sasl(VncState *vs)
+size_t vnc_client_read_sasl(VncState *vs)
 {
-    long ret;
+    size_t ret;
     uint8_t encoded[4096];
     const char *decoded;
     unsigned int decodedLen;
diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h
index b9d8de1c10..2ae224ee3a 100644
--- a/ui/vnc-auth-sasl.h
+++ b/ui/vnc-auth-sasl.h
@@ -65,8 +65,8 @@ struct VncDisplaySASL {
 
 void vnc_sasl_client_cleanup(VncState *vs);
 
-long vnc_client_read_sasl(VncState *vs);
-long vnc_client_write_sasl(VncState *vs);
+size_t vnc_client_read_sasl(VncState *vs);
+size_t vnc_client_write_sasl(VncState *vs);
 
 void start_auth_sasl(VncState *vs);
 
diff --git a/ui/vnc.c b/ui/vnc.c
index 1b5a399dc0..b0e12ca4dd 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1272,7 +1272,7 @@ void vnc_disconnect_finish(VncState *vs)
     g_free(vs);
 }
 
-ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp)
+size_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp)
 {
     if (ret <= 0) {
         if (ret == 0) {
@@ -1315,9 +1315,9 @@ void vnc_client_error(VncState *vs)
  *
  * Returns the number of bytes written, which may be less than
  * the requested 'datalen' if the socket would block. Returns
- * -1 on error, and disconnects the client socket.
+ * 0 on I/O error, and disconnects the client socket.
  */
-ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen)
+size_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen)
 {
     Error *err = NULL;
     ssize_t ret;
@@ -1335,13 +1335,13 @@ ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen)
  * will switch the FD poll() handler back to read monitoring.
  *
  * Returns the number of bytes written, which may be less than
- * the buffered output data if the socket would block. Returns
- * -1 on error, and disconnects the client socket.
+ * the buffered output data if the socket would block.  Returns
+ * 0 on I/O error, and disconnects the client socket.
  */
-static ssize_t vnc_client_write_plain(VncState *vs)
+static size_t vnc_client_write_plain(VncState *vs)
 {
     size_t offset;
-    ssize_t ret;
+    size_t ret;
 
 #ifdef CONFIG_VNC_SASL
     VNC_DEBUG("Write Plain: Pending output %p size %zd offset %zd. Wait SSF %d\n",
@@ -1442,9 +1442,9 @@ void vnc_read_when(VncState *vs, VncReadEvent *func, size_t expecting)
  *
  * Returns the number of bytes read, which may be less than
  * the requested 'datalen' if the socket would block. Returns
- * -1 on error, and disconnects the client socket.
+ * 0 on I/O error or EOF, and disconnects the client socket.
  */
-ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen)
+size_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen)
 {
     ssize_t ret;
     Error *err = NULL;
@@ -1460,12 +1460,13 @@ ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen)
  * when not using any SASL SSF encryption layers. Will read as much
  * data as possible without blocking.
  *
- * Returns the number of bytes read. Returns -1 on error, and
- * disconnects the client socket.
+ * Returns the number of bytes read, which may be less than
+ * the requested 'datalen' if the socket would block. Returns
+ * 0 on I/O error or EOF, and disconnects the client socket.
  */
-static ssize_t vnc_client_read_plain(VncState *vs)
+static size_t vnc_client_read_plain(VncState *vs)
 {
-    ssize_t ret;
+    size_t ret;
     VNC_DEBUG("Read plain %p size %zd offset %zd\n",
               vs->input.buffer, vs->input.capacity, vs->input.offset);
     buffer_reserve(&vs->input, 4096);
@@ -1491,7 +1492,7 @@ static void vnc_jobs_bh(void *opaque)
  */
 static int vnc_client_read(VncState *vs)
 {
-    ssize_t ret;
+    size_t ret;
 
 #ifdef CONFIG_VNC_SASL
     if (vs->sasl.conn && vs->sasl.runSSF)
diff --git a/ui/vnc.h b/ui/vnc.h
index 3f4cd4d93d..0c33a5f7fe 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -524,8 +524,8 @@ gboolean vnc_client_io(QIOChannel *ioc,
                        GIOCondition condition,
                        void *opaque);
 
-ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen);
-ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen);
+size_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen);
+size_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen);
 
 /* Protocol I/O functions */
 void vnc_write(VncState *vs, const void *data, size_t len);
@@ -544,7 +544,7 @@ uint32_t read_u32(uint8_t *data, size_t offset);
 
 /* Protocol stage functions */
 void vnc_client_error(VncState *vs);
-ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp);
+size_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp);
 
 void start_client_init(VncState *vs);
 void start_auth_vnc(VncState *vs);
-- 
2.14.3

  parent reply	other threads:[~2017-12-18 19:13 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-18 19:12 [Qemu-devel] [PATCH v1 00/13] Fix VNC server unbounded memory usage Daniel P. Berrange
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 01/13] ui: remove 'sync' parametr from vnc_update_client Daniel P. Berrange
     [not found]   ` <20171219072629.4c23icd27ggxayc5@starbug-vm.ie.oracle.com>
2017-12-19 10:32     ` Daniel P. Berrange
2018-01-08 11:08   ` Gerd Hoffmann
2018-01-10 13:55     ` Daniel P. Berrange
2018-01-12 11:48       ` Gerd Hoffmann
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 02/13] ui: remove unreachable code in vnc_update_client Daniel P. Berrange
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 03/13] ui: remove redundant indentation in vnc_client_update Daniel P. Berrange
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 04/13] ui: avoid pointless VNC updates if framebuffer isn't dirty Daniel P. Berrange
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 05/13] ui: track how much decoded data we consumed when doing SASL encoding Daniel P. Berrange
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 06/13] ui: introduce enum to track VNC client framebuffer update request state Daniel P. Berrange
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 07/13] ui: correctly reset framebuffer update state after processing dirty regions Daniel P. Berrange
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 08/13] ui: refactor code for determining if an update should be sent to the client Daniel P. Berrange
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 09/13] ui: fix VNC client throttling when audio capture is active Daniel P. Berrange
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 10/13] ui: fix VNC client throttling when forced update is requested Daniel P. Berrange
2017-12-19 17:57   ` Marc-André Lureau
2017-12-19 18:07     ` Daniel P. Berrange
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 11/13] ui: place a hard cap on VNC server output buffer size Daniel P. Berrange
2017-12-20 11:32   ` Marc-André Lureau
2017-12-20 11:38     ` Daniel P. Berrange
2017-12-18 19:12 ` [Qemu-devel] [PATCH v1 12/13] ui: add trace events related to VNC client throttling Daniel P. Berrange
2017-12-18 19:12 ` Daniel P. Berrange [this message]
2017-12-19  8:01 ` [Qemu-devel] [PATCH v1 00/13] Fix VNC server unbounded memory usage Darren Kenny
2017-12-19 14:57 ` Marc-André Lureau
2017-12-19 15:23   ` Daniel P. Berrange
2017-12-20 11:57 ` Marc-André Lureau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171218191228.31018-14-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=ppandit@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.