Re: [Bug-gnulib] safe-read.[ch] (safe_read): what do you think?

[Jim Meyering]

Hi Bruno,

Bruno Haible wrote:
> Jim Meyering writes:
>> I've made the following changes in the coreutils/lib for the upcoming
>> 4.5.4 release:
>>
>>      * safe-read.c (safe_read): Change type of function
>>      from ssize_t to size_t.
>>      * safe-read.h: Update prototype.
>>      (SAFE_READ_ERROR): Define.
>>
>> The question is would any of you object to my putting this change
>> in gnulib?
>
> Yes, I was going to propose just the opposite change, from size_t to
> ssize_t, in full-write.c.

But isn't it valid to call read with a length of 2^31 or larger?
In that case, read should simply read as much as it
can (as permitted by the width of ssize_t) and return
the result.  read may return a value that is smaller than the
buffer length.

> My reasoning is that safe_read is a variant of POSIX read(), which
> returns an ssize_t. It makes it easier for everyone if safe_read() has
> the same prototype as read(). [1. Easier to remember, no need for

IMHO, read's prototype is not well designed, since code using
it often ends up comparing the buffer length (of type size_t)
with the returned value (of type ssize_t).  And doing that
encourages use of a cast.

> SAFE_READ_ERROR. 2. Less error- prone when changing some code to use
> safe_read() instead of read().]

I'm hoping that serious developers will use all the tools
at their disposal (including gcc's warning options), and that
should help avoid many such problems.

>> It looks tiny, but IMHO is important, and required careful examination of
>> each use of safe_read.  There were numerous uses that compared variables
>> of type size_t and ssize_t (the old return value).
>
> For the read() system call, the code
>
>      size_t nbytes = ...;
>      if (read (fd, buf, nbytes) != nbytes)
>
> is indeed wrong, because if nbytes is > 2^31 and the read succeeds,
> read()'s return value will be
>
>      (ssize_t) nbytes
>
> not nbytes. Therefore I think the correct idiom is
>
>      size_t nbytes = ...;
>      if (read (fd, buf, nbytes) != (ssize_t) nbytes)

But what if nbytes is 2^32 - 1?
Then on some systems, the RHS will evaluate to -1.
That doesn't seem right.
Besides, the above idiom would require that every use
of safe_read have a similar cast.

The important thing for me is to keep the code (that
calls functions like safe_read) clean, and that means
avoiding casts whenever possible.  I'm not too worried
about someone calling safe_read with a 2GB buffer.

> And the same reasoning then goes for safe_read().
>
> Bruno