Agree. This looks ancient.

Grouping this together with #138258.

*** Bug 138258 has been marked as a duplicate of this bug. ***

I have updated this partly in the HEAD branch, but it needs more work, and more
investigation to see what it should actually do. The example is at least now in
the examples/book/input directory instead of being inline.

Created attachment 33905 [details] [review]
New version of the example code.

This version uses a Glib::IOChannel, and works. Anyway, I don't like the way we
attach the signal. Maybe it will be more clean to do it with the add_watch()
method of the Glib::IOChannel.

Please check out the comments for spelling and grammar, as i am not a native
english speaker.

Is this a patch after my changes?

What exactly does this do differently, and why is it better than what is there now?

When I sent the patch the existent example only compiled, so it's not a patch
after your changes (check the dates). 

I see now in HEAD a working example. I guess that the main difference is that my
patch uses a Glib::IOChannel to deal with the buffer, instead of <unistd.h>
read() in the patch by Chris Vine.

Appart of that, the patches seems to be the same.

That sounds interesting. I'd like to see an updated patch, please.

Created attachment 34592 [details] [review]
updated patch --  uses Glib::IOChannel

This updates the patch by Chris Vine; using a Glib::IOChannel to deal with the
buffer.

My toy example showed how to connect a non-blocking file descriptor into the
main program loop, which is something anyone implementing something like a
single threaded socket server (or one in which one thread deals with more
than one connection) is going to want to do.  I think it is worth keeping it as
an example of that.

By all means include the IOChannel stream buffer example as well, but
Glib::IOChannel stream buffers raise different issues when included in a C++
program.  First, they are only really suitable for sending and receiving simple
text, given the methods for reading into Glib::ustring objects and the fact that
by default codeset conversion is applied automatically.   (For transmission of
non-text data, codeset conversion can of course be turned off and
Glib::IOChannel::read(char* buf, gsize count, gsize& bytes_read) used to read
into a straight char buffer, but this is just a wrapper for Unix/Windows ::read
and therefore fairly pointless.)

Secondly, I don't think Glib::IOChannel is very useful for text either, at
least in a C++ program, as it duplicates C++ streams less well.  It will carry
out automatic code conversion and will stuff the result into a Glib::ustring and
is OK for that, but it appears that it can only (a) read up to a fixed number of
"bytes" into a Glib::ustring object (although I assume the documentation really
means "characters" as otherwise the encoding can be broken),  (b) read up to a
new line character and put it into a Glib::ustring object (c) read to EOF and
put that into a Glib::ustring object.  As far as I can see it will not for
example read individual white space separated words into a string.

The way C++ intends this to be done is by using the stream buffer classes
provided by standard C++ in conjunction (if wanted) with the iostream text
formatting functions and/or std::getline() and the like.  There are plenty
of suitable stream buffer classes available for download for use with file
descriptors - for a simple example for narrow (char) encoding schemes such as
ASCII, ISO8559 or UTF-8, see my modest offerings at
http://cvs.sourceforge.net/viewcvs.py/*checkout*/efax-gtk/efax-gtk/src/fdstream/fdstream.h?rev=1.4
and
http://cvs.sourceforge.net/viewcvs.py/*checkout*/efax-gtk/efax-gtk/src/fdstream/fdstream.cpp?rev=1.4
.

Best of all, why not include suitable stream buffers derived from
std::streambuf in glibmm itself?  If automatic code conversion is wanted
that can be done by providing a codecvt facet for the relevant locale and
imbue()ing that, although I have never come across a case where that was
useful.

As regards the Glib::IOChannel example itself, I suggest the example code should
be changed to open the fifo in blocking mode (Glib::IOChannel::set_flags()
appears to be capable of setting a flag to set the channel to read in
non-blocking mode, but since there no guarantees are made about, for example,
the size of the data packets sent and received over sockets, I doubt that can
successfully be combined with automatic codeset conversion very easily, but
maybe Glib::IOChannel does something clever).  Non-blocking mode will probably
work in practice with fifos for sending encoded text provided the text is well
under PIPE_BUF in size, but it is best not to rely on this.

Created attachment 34920 [details]
An implementation of a streambuffer using Glib::IOChannel

Murray,

I have now prepared a standards conforming stream buffer implemented using
Glib::IOChannel.  This is attached with two example programs.  One demonstrates
what it can do, and the other is a reimplementation of the example program in
gtkmm I sent you which reads and writes to a fifo.

All the buffering is done by Glib::IOChannel, which makes the implementation
quite simple.  There is a two character read buffer, but that is in order to
comply with the standard in having at least one character available for putback
and one character available for peeking.  The slight disadvantage is that
std::istream::readsome() (and similarly is_avail()) will not be of much use, as
there will only be a character available for them if there has been a peek or a
putback.  However, the standard does not require readsome() to provide anything
- it only requires it to return without blocking, and if it can fetch something
from the buffer on the way without any system reads then to provide it.

If you are interested in making more of it (say putting it in Glib) let me know
and I will put it in a form suitable for that purpose, including giving it a new
name.  There is probably some refinements I could come up with if I thought
about it.  It is a considerable improvement on Glib::StreamIOChannel.

Chris

Thanks Chris. I think I'll put this example in glib/examples (after changing it
slightly to use the existing coding style), as an example of something more
advanced. Then we can point people to it and ask for discussion. It doesn't mean
much to me so far, just because I don't use IOChannel anyway. If you then think
it's a good idea to add it to Glib, then you can make the case for it, saying
what it's good for, compared to some existing alternative. Ideally, Daniel
Elstner would take a look.

Claudio, I'll get around to processing your patch soon. Sorry.

That's fine.  It would be rather different from the rest of glibmm anyway, as it
is not just a wrapper of the C library.  The main case for it is that (a) it is
a frequent moan that there is no standard way of attaching a file descriptor to
a [io]stream given that it is so frequently needed (it is not even possible to
create a safe temporary file with standard C++) (b) there are streambuffers for
file descriptors available, but glibmm has the advantage of a certain degree of
platform independence - Glib::IOChannel will take some windows file descriptors
and windows sockets and (c) using streambuffers is the right way to do it in C++.

It would probably be worth adding a detach() function and I want to check that
all the GlibIOChannel errors are correctly handled.  If you put the example in
the examples directory I will check it out of CVS and send you any necessary
patches.

Chris.

Chris, your example is now in glibmm's cvs, in the HEAD branch.

Claudio, I applied your patch to HEAD.

I have now been through the error handling.  One issue is that
Glib::IOChannel::read(char*, gize, gsize&) can provide a read count of 0 even if
the stream is in a good state, if a UTF-8 character has been received which
cannot fit in the buffer.  This is incompatible with std::streambuffer, so I
have taken out the convert parameter to the constructor of fdstream and to
attach().  No code conversion will be carried out by the stream buffer - which
is correct anyway as operators << and >> for Glib::ustream will do the conversion.

I have also added a fdstream::get_error() function so that the additional stream
error information provided by Glib::IOChannel can be extracted if wanted by the
user, and corrected an error in fdstreambuf::xsgetn().

A patch against the earlier attachment I sent is provided giving effect to this.

Chris

Created attachment 35017 [details] [review]
patch for fdstream

Chris, please make a patch against cvs, and please put it in a more suitable
bugzilla bug.