[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: |
Jim Meyering |
Subject: |
Re: oops? read/write vs type of length parameter |
Date: |
Wed, 13 Apr 2011 08:31:09 +0200 |
Paul Eggert wrote:
> On 04/12/2011 08:53 AM, Paul Eggert wrote:
>> I can do it, in the next day or two.
>
> OK, I did it, by auditing all uses of emacs_read and emacs_write to
> find and fix all negative buffer sizes. However, instead of negative
> sizes I found overflows and some other issues, described below; and I
> therefore now agree with Eli that emacs_write shouldn't use the same
> signature as POSIX 'write'. However, I also agree with Jim that
> emacs_write's API need not artificially limit buffer sizes to SSIZE_MAX.
>
> 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. Jim, this idea is
> already used by gnulib's full_write function, so there's good
> precedent for departing from the POSIX signature here. And Eli, this
> avoids the problem where the size is so large that a signed return
> value would overflow and become negative.
>
> Here are some other bugs in Emacs that I found as part of this audit:
>
> * 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.
>
> * send_process attempts to stuff a size into an 'int' as well. This
> can be fixed by changing one of its local variables from int to size_t.
>
> * emacs_gnutls_write has the same issues as emacs_write, and should
> be fixed the same way.
>
> Attached is a bzr bundle of two proposed patches to fix the problem as
> described above. Comments are welcome.
That looks like a fine change.
Thanks for doing the work.
The improved ease of use makes this change worthwhile to me.
No more hassling with a mix of signed and unsigned lengths,
and thus less risk of misuse and fewer compiler warnings.
- Re: oops? read/write vs type of length parameter, (continued)
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/11
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/11
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/12
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/12
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/12
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/12
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/12
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/12
- Re: oops? read/write vs type of length parameter, Juanma Barranquero, 2011/04/12
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/13
- Re: oops? read/write vs type of length parameter,
Jim Meyering <=
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/13
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/13
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/13
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/13
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/13
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/13
- Re: oops? read/write vs type of length parameter, PJ Weisberg, 2011/04/13
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/14
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/13
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/13