Merge lp:~jan-kneschke/mysql-proxy/fdlimit-int-ranges into lp:mysql-proxy

[Jan Kneschke]

setmaxstdio() wants a int, setrlimit() wants unsigned int of 32 or 64 size.

This patch checks that we don't cut off bits or treat negative values are large positive values and get a false positive set().

[Leith (mleith)]

Looks good to me.

review: Approve

Unmerged revisions

1132. By <email address hidden> on 2010-09-30

added a test for chassis_fdlimit_set() and check that we don't try to set values what would
be interpreted wrongly

=== modified file 'src/chassis-limits.c'
--- src/chassis-limits.c	2010-04-06 14:26:51 +0000
+++ src/chassis-limits.c	2010-09-30 13:39:45 +0000
@@ -35,6 +35,7 @@
 #include <stdio.h> /* for _getmaxstdio() */
 #endif
 #include <errno.h>
+#include <stdint.h>
 
 #include "chassis-limits.h"
 
@@ -60,13 +61,11 @@
 	return chassis_fdlimit_set(max_files_number);
 }
 
+#ifdef _WIN32
 /**
- * set the upper limit of open files
- *
- * @return -1 on error, 0 on success
+ * set the max-open-files limit on win32
  */
-int chassis_fdlimit_set(gint64 max_files_number) {
-#ifdef _WIN32
+static int chassis_fdlimit_set_win32(int max_files_number) {
 	int max_files_number_set;
 
 	max_files_number_set = _setmaxstdio(max_files_number);
@@ -79,7 +78,12 @@
 	}
 
 	return 0;
+}
 #else
+/**
+ * set the max-open-files limit on unix
+ */
+static int chassis_fdlimit_set_unix(guint64 max_files_number) {
 	struct rlimit max_files_rlimit;
 	rlim_t soft_limit;
 	rlim_t hard_limit;
@@ -91,6 +95,31 @@
 	soft_limit = max_files_rlimit.rlim_cur;
 	hard_limit = max_files_rlimit.rlim_max;
 
+	/**
+	 * as max_files_number a int64 and rlim_t is either a 
+	 * 32bit or 64bit value we have to check if we our out of range before
+	 * we pass it down to not
+	 *
+	 * on macosx 10.6 it is a __uint64_t
+	 * on linux it is a __ULONGWORD_TYPE which may be 32bit or 64bit 
+	 */
+	switch (sizeof(rlim_t)) {
+	case 8:
+		/* our size matches, just pass it on */
+		break;
+	case 4:
+		if (max_files_number > UINT32_MAX) {
+			/* out of range */
+			return -1;
+		}
+		break;
+	default:
+		g_error("%s: we expected rlim_t to be either 8 or 4, but here it is %lu",
+				G_STRLOC,
+				sizeof(rlim_t));
+		break;
+	}
+
 	max_files_rlimit.rlim_cur = max_files_number;
 	if (hard_limit < max_files_number) { /* raise the hard-limit too in case it is smaller than the soft-limit, otherwise we get a EINVAL */
 		max_files_rlimit.rlim_max = max_files_number;
@@ -101,6 +130,29 @@
 	}
 
 	return 0;
+}
+#endif
+
+/**
+ * set the upper limit of open files
+ *
+ * @return -1 on error, 0 on success
+ */
+int chassis_fdlimit_set(gint64 max_files_number) {
+#ifdef _WIN32
+	/* make sure we don't cut off the high bits */
+	if (max_files_number > INT_MAX) {
+		return -1;
+	}
+
+	return chassis_fdlimit_set_win32((int) max_files_number);
+#else
+	/* want to have a uint, but have a int. Make sure negative values don't map to large unsigned ones */
+	if (max_files_number < 0) {
+		return -1;
+	}
+
+	return chassis_fdlimit_set_unix((guint64) max_files_number);
 #endif
 }
 

=== modified file 'tests/unit/CMakeLists.txt'
--- tests/unit/CMakeLists.txt	2010-09-27 15:54:18 +0000
+++ tests/unit/CMakeLists.txt	2010-09-30 13:39:45 +0000
@@ -28,6 +28,17 @@
 	${GTHREAD_LIBRARIES}
 	${GMODULE_LIBRARIES} 
 )
+
+ADD_EXECUTABLE(t_chassis_limits
+	t_chassis_limits.c 
+)
+TARGET_LINK_LIBRARIES(t_chassis_limits 
+	mysql-chassis
+	${GLIB_LIBRARIES}
+	${GTHREAD_LIBRARIES}
+	${GMODULE_LIBRARIES} 
+)
+
 ADD_EXECUTABLE(check_plugin
 	check_plugin.c 
 	../../src/chassis-plugin.c
@@ -166,6 +177,7 @@
 set_property(TARGET check_chassis_log check_plugin check_mysqld_proto
 	check_loadscript check_chassis_path check_chassis_filemode
 	t_network_injection t_network_backend t_network_queue
+	t_chassis_limits
 		APPEND PROPERTY COMPILE_DEFINITIONS "mysql_chassis_proxy_STATIC"
 		COMPILE_DEFINITIONS "mysql_chassis_STATIC")
 ENDIF(WIN32)
@@ -181,4 +193,5 @@
 ADD_TEST(check_chassis_filemode check_chassis_filemode)
 ADD_TEST(t_network_injection t_network_injection)
 ADD_TEST(t_network_backend t_network_backend)
+ADD_TEST(t_chassis_limits t_chassis_limits)
 

=== modified file 'tests/unit/Makefile.am'
--- tests/unit/Makefile.am	2010-09-27 15:54:18 +0000
+++ tests/unit/Makefile.am	2010-09-30 13:39:45 +0000
@@ -34,6 +34,7 @@
 	t_network_mysqld_packet \
 	t_network_mysqld_type \
 	t_network_mysqld_masterinfo \
+	t_chassis_limits \
 	t_chassis_timings \
 	t_chassis_shutdown_hooks \
 	check_chassis_filemode \
@@ -53,6 +54,17 @@
 t_chassis_shutdown_hooks_LDADD = $(GLIB_LIBS) $(GTHREAD_LIBS)
 
 
+t_chassis_limits_SOURCES = t_chassis_limits.c \
+	$(top_srcdir)/src/chassis-limits.c \
+	$(top_srcdir)/src/glib-ext.c
+
+t_chassis_limits_CPPFLAGS = \
+	-I$(top_srcdir)/src/ $(GLIB_CFLAGS) -I$(top_srcdir) \
+	$(MYSQL_CFLAGS)
+
+t_chassis_limits_LDADD = $(GLIB_LIBS) $(GTHREAD_LIBS)
+
+
 check_chassis_filemode_SOURCES = check_chassis_filemode.c \
 	$(top_srcdir)/src/chassis-filemode.c
 

=== added file 'tests/unit/t_chassis_limits.c'
--- tests/unit/t_chassis_limits.c	1970-01-01 00:00:00 +0000
+++ tests/unit/t_chassis_limits.c	2010-09-30 13:39:45 +0000
@@ -0,0 +1,53 @@
+/* $%BEGINLICENSE%$
+ Copyright (c) 2009, 2010, Oracle and/or its affiliates. All rights reserved.
+
+ This program is free software; you can redistribute it and/or
+ modify it under the terms of the GNU General Public License as
+ published by the Free Software Foundation; version 2 of the
+ License.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ 02110-1301  USA
+
+ $%ENDLICENSE%$ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <glib.h>
+
+#include "chassis-limits.h"
+
+/**
+ * try if we can set the fdlimit to a reasonable value
+ */
+void t_limits_set_fdlimit(void) {
+	g_assert_cmpint(0, ==, chassis_fdlimit_set(128));
+}
+
+/**
+ * try if out-of-range errors are handled correctly
+ */
+void t_limits_set_fdlimit_out_of_range(void) {
+	g_assert_cmpint(-1, ==, chassis_fdlimit_set(INT_MAX));
+	g_assert_cmpint(-1, ==, chassis_fdlimit_set(-1));
+}
+
+int main(int argc, char **argv) {
+	g_test_init(&argc, &argv, NULL);
+	g_test_bug_base("http://bugs.mysql.com/");
+
+	g_test_add_func("/chassis/limits/set_fdlimit", t_limits_set_fdlimit);
+	g_test_add_func("/chassis/limits/set_fdlimit_out_of_range", t_limits_set_fdlimit_out_of_range);
+
+	return g_test_run();
+}
+