Re: oops? read/write vs type of length parameter

[Jim Meyering]

Eli Zaretskii wrote:

>> From: Jim Meyering <address@hidden>
>> Date: Mon, 11 Apr 2011 10:55:35 +0200
>> Cc: Eli Zaretskii <address@hidden>
>>
>> In http://bzr.savannah.gnu.org/lh/emacs/trunk/revision/103883
>> you adjusted the signatures of emacs_read and emacs_write.
>>
>>   - extern int emacs_read (int, char *, unsigned int);
>>   - extern int emacs_write (int, const char *, unsigned int);
>>   + extern ssize_t emacs_read (int, char *, ssize_t);
>>   + extern ssize_t emacs_write (int, const char *, ssize_t);
>
> Yes, that's part of my on-going effort to allow editing of files
> larger than 2GB.  With that revision, I can finally visit such a large
> file, modify it, and save it to disk :-)
>
>> It's good to see that you corrected the return type to be wider
>> (ssize_t) and still signed, just like "int".
>
> That's what its callers expect: the return value can be positive or
> negative.

As I said, that part is welcome.
It also makes these functions more like read and write.

>> However, did you really intend to make the buffer length parameters signed?
>> I would have expected those to be of type size_t, not ssize_t.
>
> We call these functions with an argument of type EMACS_INT, which can
> be negative.

With read and write, there is already a tension there,
since the return value (length or error indicator) is signed,
while the length parameter is unsigned, and hence slightly wider.

> I don't want it to wind up as a large positive value
> inside these functions, I'd rather the functions fail instead.  Note
> that emacs_write is careful enough to check the sign of that argument,
> and if we want a similar guard in emacs_read, we can easily add that.

Currently, emacs_write silently ignores an invalid buffer length,
treating it just like a length of 0.  It'd be better not to ignore
such an error.

IMHO, an interface that takes a logically unsigned parameter
should have an unsigned type.

I guess I'm biased towards least-surprise for developers, so I
think read- and write-like functions should accept a buffer length
argument of type size_t, to be consistent with read and write.
To try to protect against bugs by changing API to a signed type may
actually cause trouble when callers end up mixing/comparing their
newly signed (to accommodate the invented API) and unsigned lengths
from standard functions.

Another thing to keep in mind: on some older systems, trying to read
more than INT_MAX bytes in a single syscall will fail.