On Fri, Jul 27, 2018 at 4:07 AM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>>
>> 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.
Sorry for the slow response. I'll have to look at this again. I
think I wanted to avoid terminating the program, so the skipping of
g_error was important for that. But I can't recall, so I'll have to
re-investigate.
Regards,
Jason
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel