Fix handling of free descriptors when calling dup2.

native_client_sdk/src/tests/nacl_io_test/kernel_proxy_test.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <errno.h>
 #include <fcntl.h>
 #include <pthread.h>
 #include <stdio.h>
 #include <sys/stat.h>
 #include <sys/types.h>
@@ skipping 509 matching lines @@
   ASSERT_EQ(4, ki_write(dup_fd, "more", 4));
 
   ASSERT_EQ(0, ki_close(dup2_fd));
   // fd, new_fd -> "/bar"
   // dup_fd -> "/foo"
 
   ASSERT_EQ(dup_fd, ki_dup2(fd, dup_fd));
   // fd, new_fd, dup_fd -> "/bar"
 }
 
+TEST_F(KernelProxyTest, Dup2Allocation) {

[Sam Clegg, 2014/12/12 00:44:40]

Comment here?   I would like a better test name but I can see its hard to come
up with a short and more meaningful one.

[bradn, 2014/12/12 22:58:18]

Added more.

+  int fd = ki_open("/foo", O_CREAT | O_RDWR, 0777);
+  ASSERT_GT(fd, -1);
+
+  int dup_fd = ki_dup(fd);
+  ASSERT_EQ(fd + 1, dup_fd);

[binji, 2014/12/12 18:55:20]

Is this guaranteed to be true? The fd returned should always be the lowest
available, right? In that case, if 3 handles are opened, and the middle one is
closed, then this would be false, correct?

Maybe it would be safer to allocate a bunch of fds until we get three in a row,
at which point those can be released, and we know it's safe to continue the
test.

Or am I just being paranoid?

[bradn, 2014/12/12 22:58:18]

Commented more verbosely.
The test assumes the initial descriptor set starts from zero and is contiguous.
I suppose that might strictly be true (though if anything I would imagine posix
probably mandates something about only 0-2 being initially allocated.
I've commented more extensively.
The trouble with trying to find an contiguous region is that the fix this is
meant to cover might have hidden behind allocating the descriptors one by one
(as then they'd correctly be in the free list).
I suppose grabbing bigger chunks might work, but that seems overkill.

+
+  ASSERT_EQ(100, ki_dup2(fd, 100));
+
+  int dup_fd2 = ki_dup(fd);
+  ASSERT_EQ(fd + 2, dup_fd2);
+}
+
 TEST_F(KernelProxyTest, Lstat) {
   int fd = ki_open("/foo", O_CREAT | O_RDWR, 0777);
   ASSERT_GT(fd, -1);
   ASSERT_EQ(0, ki_mkdir("/bar", S_IREAD | S_IWRITE));
 
   struct stat buf;
   EXPECT_EQ(0, ki_lstat("/foo", &buf));
   EXPECT_EQ(0, buf.st_size);
   EXPECT_TRUE(S_ISREG(buf.st_mode));
 
@@ skipping 465 matching lines @@
 
   int fd = ki_open("/dummy", O_RDONLY, 0);
   EXPECT_NE(0, fd);
 
   char buf[20];
   EXPECT_EQ(-1, ki_read(fd, &buf[0], 20));
   // The Filesystem should be able to return whatever error it wants and have it
   // propagate through.
   EXPECT_EQ(1234, errno);
 }