> > On Wed, Jul 25, 2018 at 5:53 AM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: > >> > >> The GIOStatus return value indicates errors, so catch it and check it to > >> know if we should return an error. > >> > >> Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx> > >> --- > >> Truth be told, I can't remember exactly why I had to write this. Could > >> status be G_IO_STATUS_ERROR, but err still be NULL? Since I added > >> debugging output with those value, it maybe the case. > >> > > > > Looking at GLib source code there are different paths leading to > > status == G_IO_STATUS_ERROR and err == NULL. > > For instance trying to mix read and write and encoding. > > Or for some parameter checks. > > > >> src/vscclient.c | 11 ++++++++++- > >> 1 file changed, 10 insertions(+), 1 deletion(-) > >> > >> diff --git a/src/vscclient.c b/src/vscclient.c > >> index fa60162..56e2ced 100644 > >> --- a/src/vscclient.c > >> +++ b/src/vscclient.c > >> @@ -69,12 +69,21 @@ do_socket_send(GIOChannel *source, > >> { > >> gsize bw; > >> GError *err = NULL; > >> + GIOStatus status; > >> > >> g_return_val_if_fail(socket_to_send->len != 0, FALSE); > >> g_return_val_if_fail(condition & G_IO_OUT, FALSE); > >> > >> - g_io_channel_write_chars(channel_socket, > >> + status = g_io_channel_write_chars(channel_socket, > >> (gchar *)socket_to_send->data, socket_to_send->len, &bw, &err); > >> + if (verbose) { > >> + printf("status: %d bytes written: %d err: %p\n", status, bw, > >> err); > > > > This looks like more a debug thing than a verbose one (don't know > > much about libcacard). > > vscclient doesn't have a debug option, but it does have some verbose > > 10 checks for debug-like messages. I can bump it up to that. > Looking better libcacard is using GLib, maybe a g_debug would be better. > >> + } > >> + > >> + if (status == G_IO_STATUS_ERROR) { > > > > No logs? > > I can add one. Also, I'm testing out re-ordering this test after the > err != NULL check. In either case we'll abort from the function, but > if err is set, then we can print a more informative message. > Make sense. > >> + return FALSE; > >> + } > >> + > >> if (err != NULL) { > >> g_error("Error while sending socket %s", err->message); So any error here with a message is terminating the program. > >> return FALSE; And this is never executed. > > > > Frediano > > Thanks for the review. > > Jason > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel