[swrap] [PATCH] treat newfd correctly in dup2()

[Michael Adam]

Hi,

attached find a patch that fixes our treatment of 'newfd'
in swrap_dup2() that Metze and I wrote.

Anoop has written a test case for that (it fails without
the swrap patch). Thanks!

Review / push appreciated!

Michael
-------------- next part --------------
From 3cf712966ed06a067a11bbd5835781de625edd96 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 16 Aug 2016 10:59:40 +0200
Subject: [PATCH 1/2] swrap: treat the case of fd == newfd correctly in dup2()

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 src/socket_wrapper.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
index 3dc80fa..6c4ae3b 100644
--- a/src/socket_wrapper.c
+++ b/src/socket_wrapper.c
@@ -5181,6 +5181,16 @@ static int swrap_dup2(int fd, int newfd)
 		return libc_dup2(fd, newfd);
 	}
 
+	if (fd == newfd) {
+		/*
+		 * According to the manpage:
+		 *
+		 * "If oldfd is a valid file descriptor, and newfd has the same
+		 * value as oldfd, then dup2() does nothing, and returns newfd."
+		 */
+		return newfd;
+	}
+
 	if (find_socket_info(newfd)) {
 		/* dup2() does an implicit close of newfd, which we
 		 * need to emulate */
-- 
2.5.5


From a89f5c7e7492b1fd67e74740800ff5ae3a8315b7 Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Mon, 22 Aug 2016 19:37:47 +0530
Subject: [PATCH 2/2] swrap: Add test case to validate oldfd = newfd case in
 dup2()

Signed-off-by: Anoop C S <anoopcs at redhat.com>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 tests/CMakeLists.txt  |  1 +
 tests/test_tcp_dup2.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 tests/test_tcp_dup2.c

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index aecf6b8..9b5c4bf 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -21,6 +21,7 @@ target_link_libraries(${TORTURE_LIBRARY}
 set(SWRAP_TESTS
     test_ioctl
     test_tcp_listen
+    test_tcp_dup2
     test_echo_tcp_socket
     test_echo_tcp_connect
     test_echo_tcp_bind
diff --git a/tests/test_tcp_dup2.c b/tests/test_tcp_dup2.c
new file mode 100644
index 0000000..fd6adc2
--- /dev/null
+++ b/tests/test_tcp_dup2.c
@@ -0,0 +1,50 @@
+#include "torture.h"
+
+#include <cmocka.h>
+#include <unistd.h>
+
+static int setup(void **state)
+{
+	torture_setup_socket_dir(state);
+
+	return 0;
+}
+
+static int teardown(void **state)
+{
+	torture_teardown_socket_dir(state);
+
+	return 0;
+}
+
+static void test_dup2_existing_open_fd(void **state)
+{
+	int s, dup_s;
+
+	(void) state; /* unused */
+
+	s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+	assert_int_not_equal(s, -1);
+
+	/*
+	 * Here we try to duplicate the existing socket fd to itself
+	 * and as per man page for dup2() it must return the already
+	 * open fd without any failure.
+	 */
+	dup_s = dup2(s, s);
+	assert_int_equal(dup_s, s);
+
+	close(s);
+}
+
+int main(void) {
+	int rc;
+
+	const struct CMUnitTest tcp_dup2_tests[] = {
+		cmocka_unit_test(test_dup2_existing_open_fd),
+	};
+
+	rc = cmocka_run_group_tests(tcp_dup2_tests, setup, teardown);
+
+	return rc;
+}
-- 
2.5.5

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160823/866a3243/signature.sig>