Re: [Patch/RFC] Fix libstdc++/9533

[Nathan Myers <ncm-nospam at cantrip dot org>]

On Thu, Mar 13, 2003 at 01:54:48PM +0100, Paolo Carlini wrote:
> the below fixes the problem by implementing a non-trivial showmanyc(),
> as discussed thoroughly on the list during the last weeks.
> 
> In particular, I would appreciate by reassured that the new
> __basic_file<>::showmanyc_helper deals correctly with the various
> possibilities.

This patch needs some cleanup.  I apologize for the delay in replying;
I was sick as a dog all of last week.

> +#ifdef _GLIBCPP_HAVE_RECV_PEEK_SAFE
> +    // Pipes.
> +    streamsize __size =
> +      recv(this->fd(), 0, 0, (MSG_DONTWAIT|MSG_PEEK));
> +    if (__size >= 0)
> +      return __size;
> +#endif

First, the above code can only reveal information about sockets, and 
not, unfortunately, pipes.  (On a pipe it reports failure.)  So, the 
comment is not right.  Still, sockets are important enough to merit 
the special treatment.  

(I wish we could do pipes too, but I don't know of an equivalent 
operation for pipes, besides setting and restoring O_NONBLOCK around 
a read(), which I don't like, or using poll() followed by a regular 
read() (guaranteed not to block, on file descriptors that support
short reads), which is only a little better.  showmanyc() shouldn't
consume characters from the stream at all.)

Second, passing zeroes to the recv() arguments isn't portable.  I find
that, on Linux, (1) we must actually provide a buffer for it to copy 
the characters into, and (2) it won't return a value larger than the 
indicated buffer size (even with MSG_TRUNC).   This means you can't 
avoid having it copy the characters.  (Drat.) The buffer used might as 
well be the actual stream buffer.  This is still less expensive than 
an actual underflow, potentially, because we don't have to run the 
codecvt operation afterward.  Further, we haven't actually messed with 
the socket state.  

> +#ifdef _GLIBCPP_HAVE_S_ISREG
> +    // Regular files.
> +    struct stat __buffer;
> +    int __ret = fstat(this->fd(), &__buffer);
> +    if (!__ret && S_ISREG(__buffer.st_mode))
> +      if (__stdio)
> +	return __buffer.st_size - ftell(_M_cfile);
> +      else
> +	return __buffer.st_size - lseek(this->fd(), 0, ios_base::cur);
> +#elif _GLIBCPP_HAVE_S_IFREG
> +    // Regular files.
> +    struct stat __buffer;
> +    int __ret = fstat(this->fd(), &__buffer);
> +    if (!__ret && S_IFREG(__buffer.st_mode))
> +      if (__stdio)
> +	return __buffer.st_size - ftell(_M_cfile);
> +      else
> +	return __buffer.st_size - lseek(this->fd(), 0, ios_base::cur);
> +#endif

I would prefer to express the above as

> +#ifdef _GLIBCPP_S_ISREG_OR_S_IFREG
> +    // Regular files.
> +    struct stat __buffer;
> +    int __ret = fstat(this->fd(), &__buffer);
> +    if (!__ret && _GLIBCPP_S_ISREG_OR_S_IFREG(__buffer.st_mode))
> +      if (__stdio)
> +	return __buffer.st_size - ftell(_M_cfile);
> +      else
> +	return __buffer.st_size - lseek(this->fd(), 0, ios_base::cur);
> +#endif

with a corresponding definition for _GLIBCPP_S_ISREG_OR_S_IFREG 
constructed in configure.  Of course LARGE_FILE refinements are possible,
eventually.  First things first.

Also...

> +
> +      streamsize
> +      showmanyc_helper(bool __stdio);

As in several other patches that have come through, I believe this 
member name must be uglified.  I don't mind if they're all fixed up
in a separate patch.

Nathan Myers
ncm-nospam at cantrip dot org