[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Eli Zaretskii
Subject: Re: oops? read/write vs type of length parameter
Date: Wed, 13 Apr 2011 02:37:14 -0400

> Date: Tue, 12 Apr 2011 22:14:48 -0700
> From: Paul Eggert <address@hidden>
> CC: address@hidden, address@hidden
> However, I also agree with Jim that
> emacs_write's API need not artificially limit buffer sizes to SSIZE_MAX.

Emacs cannot have buffers (or any other streams of bytes) that are
larger than SSIZE_MAX, because a small number of bits is reserved for
the Lisp tags.  So complicating the users of emacs_write in order to
support such large writes is pointless, and targets theoretical cases
that cannot happen in Emacs.

Emacs is not a general-purpose library.  It doesn't have to cater to
use cases that can never happen without radical changes in basic
architecture and data types, that would mean a complete rewrite of the
entire package.

> Here's the key idea of the attached patch.  Since
> "N = emacs_write (FD, BUF, SIZE)" returns N if successful, and some
> value less than N if it fails, the caller can easily determine whether
> it succeeded by testing N == SIZE.  So on failure there's no need for
> emacs_write to return a special value like -1.
> With this in mind, emacs_write can simply return the number of bytes
> written, which will always be a size_t value.

This will not allow us support situations where a call to emacs_write
or similar interfaces (such as emacs_gnutls_write) can legitimately
write only a portion of what it was required to.  The current code
handles this situation (by looping for what's left to write), while
your suggested code will treat that as a fatal error.  Are we sure
that all such cases are irrecoverable errors?  If not, and if this
patch is accepted, should we perhaps test more pertinent values of
errno than what we do now, to avoid failing unnecessarily?

>  * Several places in sound.c attempt to stuff a size into an 'int',
>    which doesn't work.  This problem is independent of the above, and
>    its fix is independent of the above too, so it's split into a
>    separate patch in the attached bundle.

I didn't touch those parts because I don't know enough about the
underlying sound APIs.  Someone who does should review the proposed
changes and verify that they don't break sound support with those
devices.  We cannot assume that a given hardware device is 64-bit- and
signedness-clean as it should be.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]