Re: GIOChannel error handling

[Owen Taylor <otaylor redhat com>]

Michael Natterer <mitch gimp org> writes:

> Johannes Stezenbach <js convergence de> writes:
> 
> > The GIOError type is IMHO unsufficient for proper error
> > handling, in some cases one needs access to the real errno.
> > 
> > (E.g. reading from a device driver which can report buffer
> > under/overruns.)

Could you be more specific here about your requirements? 

In the buffering and encoding conversion patch that has been floating
around for a few months and I'm curently applying, we've deprecated
GIOError (which was pretty lame), in favor of:

typedef enum
{
  G_IO_STATUS_ERROR,
  G_IO_STATUS_NORMAL,
  G_IO_STATUS_EOF,
  G_IO_STATUS_PARTIAL_CHARS, /* like EOF, but with unconverted data left */
  G_IO_STATUS_AGAIN
} GIOStatus;

and for G_IO_STATUS_ERROR, we fill in a GError * (if provided)
with an error of one of the following types:

typedef enum
{
  /* Derived from errno */
  G_IO_CHANNEL_ERROR_ACCES,
  G_IO_CHANNEL_ERROR_BADF,
  G_IO_CHANNEL_ERROR_DEADLK,
  G_IO_CHANNEL_ERROR_FAULT,
  G_IO_CHANNEL_ERROR_INVAL,
  G_IO_CHANNEL_ERROR_IO,
  G_IO_CHANNEL_ERROR_ISDIR,
  G_IO_CHANNEL_ERROR_MFILE,
  G_IO_CHANNEL_ERROR_NOLCK,
  G_IO_CHANNEL_ERROR_NOSPC,
  G_IO_CHANNEL_ERROR_PERM,
  G_IO_CHANNEL_ERROR_PIPE,
  G_IO_CHANNEL_ERROR_SPIPE,
  /* Other */
  G_IO_CHANNEL_ERROR_ENCODE_RW,
  G_IO_CHANNEL_ERROR_FAILED
} GIOChannelError;

With other errors being binned into G_IO_CHANNEL_ERROR_FAILED
(Current set is basically random - could be tweaked a bit)

But:

 a) certainly we aren't going to have every possible errno
    have a separate value in GIOChannelError

 b) The case where a GError is returned is a slow error case - 
    it involves building human readable strings.

ALso, remember that there is no real advantage of calling
g_io_channel_read()/write() for raw reads unless you want the extra
abstraction layer. Using the syscalls directly in conjunction with
g_io_add_watch() is legitimate.

(There may be some small fixups necessary for this in the
patch and G_IO_OUT, but it is definitely something that 
needs to be preserved.)

I'm not sure it makes any sense to require g_io_channel_read()
on a Unix fd to act exactly like read() on the fd - what
would be the point?

> > In my code I currently just check errno directly when I get
> > G_IO_ERROR_UNKNOWN, which works with the current GIOChannel
> > implementation. However, the Glib API does not guarantee that
> > this is valid, and IMHO it also is somewhat unclean.
> > 
> > 
> > I therefore propose to add
> > 
> >   gint g_io_channel_unix_get_errno (GIOChannel *channel);
> > 
> > implemented by adding an "errno" member to struct _GIOUnixChannel
> > and setting it directly after every system call done in giounix.c.



> > I don't know if the concept of errno is really OS indenpendent,
> > so I propose it for UNIX only. It would make sense to provide
> > something similar for other OSs, too, though. Maybe it should
> > even be
> > 
> >   gint g_io_channel_get_errno (GIOChannel *channel);
> > 
> > 
> > Opinons?
> 
> Conceptually, this is the same bug that GTK+ 1.2 suffered from regarding
> "EINTR". This was fixed in GLIB HEAD by adding G_IO_ERROR_AGAIN. We will
> use this in GIMP 1.4 instead of the ugly direct access to "errno".

Well, or not ;-) In the buffering/encoding covnersion patch, 
I simply made the Unix IO channels retry on EINTR. I'm
of the opinion that EINTR is a very much a low-level, annoying,
Unix detail, and it has no place in a "generic" API like
GIOChannel. 

If someone really knows that they want their IO operations to be
interrupted by random incoming signals, then nothing is stopping them
from using read()/write() on the file descriptor directly
(as long as you don't mix that with buffered operations of
course.)
 
> Given the fact that device drivers or strange OSs may set errno to
> whatever they consider appropriate, we should IMHO follow Johannes'
> proposal and make the original errno available through an API.
> 
> I'm however biassed whether this should be g_io_channel_unix_get_errno()
> or g_io_channel_get_errno(). Tim??

g_ui_channel_unix_get_errno(), if at all.

Regards,
                                        Owen