[PATCH] [libc++] Don't return uninitialized data from random_device::operator()
Marshall Clow
mclow.lists at gmail.com
Wed May 28 07:23:17 PDT 2014
On May 27, 2014, at 11:44 AM, David Majnemer <david.majnemer at gmail.com> wrote:
> On Tue, May 27, 2014 at 7:45 AM, Marshall Clow <mclow.lists at gmail.com> wrote:
>
> On May 20, 2014, at 2:25 AM, David Majnemer <david.majnemer at gmail.com> wrote:
>
>> Oops, sent out the wrong version of this patch. Attached is what I intended to send.
>>
>>
>> On Tue, May 20, 2014 at 12:28 AM, David Majnemer <david.majnemer at gmail.com> wrote:
>> random_device::operator() as currently implemented does not correctly handle errors returned by read. This can result in it returning uninitialized data.
>>
>> To fix this, wrap the call to read in a loop.
>
> I like this; but can you think of any way to test it?
>
> I've added a test for the EOF case.
>
> I have written a test for EINTR case but I did not included it because it's inherently not reliable. It checks the output of operator() to see if signals resulted in us getting a lot of zero return results.
I get a couple of signedness warnings when building:
+ for FILE in '../src/*.cpp'
+ /Sources/LLVM/bin/bin/clang++ -c -g -Os -arch i386 -arch x86_64 -nostdinc++ -std=c++11 -fstrict-aliasing -Wall -Wextra -Wshadow -Wconversion -Wpadded -Wstrict-aliasing=2 -Wstrict-overflow=4 -I../include ../src/random.cpp
../src/random.cpp:78:14: warning: implicit conversion changes signedness: 'ssize_t' (aka 'long') to 'unsigned long' [-Wsign-conversion]
i += s;
~~ ^
1 warning generated.
../src/random.cpp:78:14: warning: implicit conversion changes signedness: 'ssize_t' (aka 'long') to 'unsigned long' [-Wsign-conversion]
i += s;
~~ ^
1 warning generated.
I get that read can only return -1, 0, and positive numbers, and you’re already checking for -1 and 0, so I think that you can safely cast s to a size_t before doing the addition.
Also, from a readability standpoint, wouldn’t a while loop be better than the for loop?
Something like this:
while (n > 0)
{
ssize_t s = read(__f_, p, n);
if (s == 0)
__throw_system_error(ENODATA, "random_device got EOF");
if (s == -1)
{
if (errno != EINTR)
__throw_system_error(errno, "random_device got an unexpected error");
continue;
}
i -= (size_t) s;
p += (size_t) s;
}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140528/b1906747/attachment.html>
More information about the cfe-commits
mailing list