Re: Outstanding GIOChannel issues

[Owen Taylor <otaylor redhat com>]

Ron Steinke <rsteinke w-link net> writes:

> The implementation of the new GIOChannel API for 2.0 is mostly complete, but
> there are a few outstanding issues I'd like some comments on:
> 
>   * win32 implmentation:
> 
>     Currently being handled by Hans Breuer (many thanks). These include
>     bug 57689, bug 57690, bug 57691, and bug 57692. Hans, would you like
>     me to transfer ownership of these bugs to you?
> 
>     The problem in this area that worries me is that we haven't made up our
>     minds how to handle O_BINARY (bug 57695). Ideas? Opinions? Anyone?

Let's:

 * Always specify O_BINARY on platforms where it matters

 * Keep in mind the future possibility of something like:
   g_io_channel_set_newline_format (channel, "\r\n");

   This becomes most useful when we add encoding autodetection
   to glib - planned for a future version. Then we 
   can add newline detection like that, and get the effect
   of something similar to Emacs's newline handling. Emacs,
   when it reads a file, autodetects the newline format
   as part of encoding detection, so you get an encoding
   like EUC-JP-DOS, manipulates newlines internally as
   '\n' then uses the same newline format when writing
   out the file.
  
I think it is a good trait if the DOS and Unix versions of a
program by default produce identical output files. 

And not specifying O_BINARY corrupts encodings such UTF-16.

If we add the functionality to do read/write conversions
of newlines, it should happen on the UTF-8 side of the 
conversion, and should be cross-platform.

>   * character-at-a-time read:
> 
>     For variable width UTF-8 charaters, the current API
>     would require users to write code of the form:
> 
>     char buf[6];
>     GIOStatus status;
>     gsize char_size, loop;
>     GError *err = NULL;
> 
>     for (loop = 1; loop <= 6; ++loop)
>       {
>         status = g_io_channel_read_chars (channel, buf, loop, &char_size, &err);
> 
>         if (char_size != 0 || status != G_IO_STATUS_NORMAL)
>           break;
>       }
> 
>     g_assert (loop <= 6);
> 
>     /* Handle cases */
> 
>     which is ugly, and requires a lot of unnecessary looping. On the
>     other hand, it would be possible to write
> 
>     GIOStatus g_io_channel_get_unichar (GIOChannel *channel,
>                                         gunichar   *char,
>                                         GError     *error);
> 
>     into the API with minimal effort. Is this an important issue, or
>     are we too late into the API freeze? Owen?

Despite getc(), I think this should be g_io_channel_read_unichar()

If we add that, should we add add g_io_channel_write_unichar()?
though that's an even more trivial function:
 
 gchar buf[6];
 gint len = g_unichar_to_utf8 (buf, 6);
 return g_io_channel_write_chars (buf, len, NULL, &error);

I think you should go ahead and add read/write_unichar().
As you say, read_unichar() is very painful to implement in
terms of the current API, 

>   * error handling in g_io_channel_write_chars()
> 
>     I think we're going to have to allow for the possibility of partial
>     writes on the channel. In the relevant case, part of the
>     (aribitrarily large, much greater than buf_size) data has been
>     written to disk, and the next write returns G_IO_STATUS_AGAIN.
>     In this case, I think the sensible thing to do is return
>     G_IO_STATUS_NORMAL, and set bytes_written to whatever we've
>     been able to write so far (including copying to the buffer).
> 
>     If we allow partial writes, do we still want to use the partial_chars_buffer
>     for partial characters at the end of input?
> 
>     Owen, I would really like to hear your opinion on this.

Partial writes on non-blocking sockets are fine. What I've been
saying we should avoid is partial writes on blocking sockets.

Yes, we should still use a partial_chars_buffer() for partial chars in
the EGAIN case. Otherwise, e.g., the above write_unichar() function
doesn't work in the non-blocking case. Having a guarantee that an
whole number of characters appears to be written is quite useful.
 
>   * minor issues
> 
>     * public/private parts of GIOChannel:
> 
>       I've marked the close_on_unref flag public, since it's
>       safe to have users twiddle that one. Everything else
>       is marked private. The only exception that may be necessary
>       to this are the is_readable and is_writeable flags,
>       which the people at GNet may need to twiddle in
>       their wrapper for the unix shutdown() function,
>       among other places.

Just because a struct member member is marked public, doesn't mean
that it can be twiddled. The rule is that generally public members are
readonly. The reason for this is that even when reading a field
directly is "safe" there is usually accompanying state that needs
to be affected when changing the field.

What I would say is:

 - If there are fields that need to be modified by people other
   than io-channel implementations, there should be public
   accessors.

 - If we allow external implemenetations of IO Channels
   than we should add accessors for any fields they need
   to change.

 - If we don't allow external implemetnations of IO channels,
   then we should move the IOChannel struct into a private 
   header. 
 
>     * bug 52811 (the old GIOChannel API bug):
> 
>       Owen, can you close this? I don't have the necessary permissions.

Done. (And have given you the permissions to edit bug
reports you haven't filed yourself.)

Regards,
                                        Owen