[Spice-devel] [PATCH libcacard 1/2] vscclient: Catch write errors

[Jason Andryuk]

On Wed, Jul 25, 2018 at 5:53 AM, Frediano Ziglio <fziglio at redhat.com> 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 at gmail.com>
>> ---
>> 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.

>> +    }
>> +
>> +    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.

>> +        return FALSE;
>> +    }
>> +
>>      if (err != NULL) {
>>          g_error("Error while sending socket %s", err->message);
>>          return FALSE;
>
> Frediano

Thanks for the review.

Jason