Merge lp:~miguelaraujo/mysql-proxy/lib-location-hardcoding into lp:mysql-proxy/0.8

[Miguel Araújo]

fixed hardcoding of plugin location:

    - exposed EXEC_PREFIX and PLUGINDIR on CFLAGS;
    - added gboolean chassis_path_string_is_parent_of(const char *parent, const char *child), to check if a path-string is inside another;
    - added stripping of EXEC_PREFIX from PLUGINDIR if necessary;
    - added builing of filename with correct path to installed libraries;

  added unit tests for chassis_path_string_is_parent_of();

[lp:~miguelaraujo/mysql-proxy/lib-location-hardcoding]

1236. By Miguel Araújo on 2012-02-17

added missing comments for gboolean chassis_path_string_is_parent_of(const char *parent, const char *child)

1237. By Miguel Araújo on 2012-02-20

Added missing else statement on chassis_frontend_init_plugin_dir();

Extended the parameters list of chassis_path_string_is_parent_of:

  - gboolean chassis_path_string_is_parent_of(const char *parent, gsize parent_len, const char *child, gsize child_len);
  - avoids using strlen();
  - we can get the size of the string from Gstring and static strings has the length at compile time;

1238. By Miguel Araújo on 2012-02-20

fixed temporary string comparing bug;

updated unit tests for chassis_path_string_is_parent_of;

[Jan Kneschke (jan-kneschke)]

* fix mix of space vs. tab for indenting
* fix coding style:
  if (...) {
   ...
  } else {
   ...
  }
* plugin_dir = g_build_filename(g_strdup(PLUGINDIR), NULL); doesn't need a g_build_filename(), g_strdup() does the same.
* g_build_filename(base_dir, g_strdup(rel_plugin_dir), NULL); creates a mem-leak as g_strdup() creates a copy which is never freed. The g_strdup() isn't needed in g_build_filename()
* add tests for chassis_path_string_is_parent_of(C(""), C("/foo"));

[lp:~miguelaraujo/mysql-proxy/lib-location-hardcoding]

1239. By Miguel Araújo on 2012-02-20

fixed identation differences (spaces vs tabs);

fixed coding style for if/else statements;

replaced g_build_filename for g_strdup on chassis_frontend_init_plugin_dir;

removed unnecessary g_strdup on g_build_filename that leads to a mem-leak;

added missing verification if parent-path is empty on chassis_path_string_is_parent_of, and corresponding unit test;

[Jan Kneschke (jan-kneschke)]

add test for:
g_assert_cmpint(TRUE, ==, chassis_path_string_is_parent_of(C("/foo/bar/"), C("/foo/bar")));
g_assert_cmpint(FALSE, ==, chassis_path_string_is_parent_of(C("/foo/bar/"), C("/foo/bar2")));

review: Needs Fixing

[Jan Kneschke (jan-kneschke)]

referring to the previous comment:

  g_assert_cmpint(FALSE, ==, chassis_path_string_is_parent_of(C("/foo/bar/"), C("/foo/bar")));

as they the same, but not parent/child.

[lp:~miguelaraujo/mysql-proxy/lib-location-hardcoding]

1240. By Miguel Araújo on 2012-02-21

added missing check for child lengh in order to compare correctly with parent;

added tests for:

  - child with same lenght as parent but different;
  - same but not parent( '/foo/bar/', '/foo/bar' );

1241. By Miguel Araújo on 2012-02-21

removed unnecessary g_assert_cmpint() on chassis_frontend_init_plugin_dir();

removed unncessary else statement on chassis_path_string_is_parent_of();

1242. By Miguel Araújo on 2012-02-21

optimizations on chassis_path_string_is_parent_of();

1243. By Miguel Araújo on 2012-02-21

added missing EXEC_PREFIX and PLUGINDIR exposed on CFLAGS;

fixed swapped vars with checking if exec-dir is parent of plugin-dir on chassis_frontend_init_plugin_dir();

[Jan Kneschke (jan-kneschke)]

looks good.

review: Approve

=== modified file 'src/Makefile.am'
--- src/Makefile.am	2011-11-14 16:26:34 +0000
+++ src/Makefile.am	2012-02-21 13:12:19 +0000
@@ -22,7 +22,7 @@
 
 
 BUILD_CPPFLAGS = $(LUA_CFLAGS) $(MYSQL_CFLAGS) $(GLIB_CFLAGS) $(GMODULE_CFLAGS) $(GTHREAD_CFLAGS)
-BUILD_CFLAGS   = -DLUAEXTDIR="\"$(luaextdir)\""  -DPLUGINDIR="\"$(plugindir)\""
+BUILD_CFLAGS   = -DLUAEXTDIR="\"$(luaextdir)\""  -DPLUGINDIR="\"$(plugindir)\""  -DEXEC_PREFIX="\"$(exec_prefix)\""
 BUILD_LDADD    = $(GLIB_LIBS) $(GMODULE_LIBS) libmysql-chassis.la $(GTHREAD_LIBS) libmysql-proxy.la libmysql-chassis-glibext.la
 
 BUILT_SOURCES =
@@ -99,7 +99,7 @@
 
 
 libmysql_chassis_la_LDFLAGS  = -export-dynamic -no-undefined -dynamic
-libmysql_chassis_la_CPPFLAGS = $(MYSQL_CFLAGS) $(EVENT_CFLAGS) $(GLIB_CFLAGS) $(LUA_CFLAGS) $(GMODULE_CFLAGS) $(GTHREAD_CFLAGS)
+libmysql_chassis_la_CPPFLAGS = $(MYSQL_CFLAGS) $(EVENT_CFLAGS) $(GLIB_CFLAGS) $(LUA_CFLAGS) $(GMODULE_CFLAGS) $(GTHREAD_CFLAGS) -DPLUGINDIR="\"$(plugindir)\""  -DEXEC_PREFIX="\"$(exec_prefix)\""
 libmysql_chassis_la_LIBADD   = $(EVENT_LIBS)   $(GLIB_LIBS)   $(LUA_LIBS) $(GMODULE_LIBS) $(GTHREAD_LIBS) libmysql-chassis-timing.la libmysql-chassis-glibext.la
 
 lib_LTLIBRARIES += libmysql-proxy.la

=== modified file 'src/chassis-frontend.c'
--- src/chassis-frontend.c	2012-01-23 20:01:49 +0000
+++ src/chassis-frontend.c	2012-02-21 13:12:19 +0000
@@ -308,11 +308,25 @@
 
 int chassis_frontend_init_plugin_dir(char **_plugin_dir, const char *base_dir) {
 	char *plugin_dir = *_plugin_dir;
+	char *rel_plugin_dir;
 
 	if (plugin_dir) return 0;
 
 #ifdef WIN32
 	plugin_dir = g_build_filename(base_dir, "bin", NULL);
+
+#elif defined(PLUGINDIR) && defined(EXEC_PREFIX)
+
+	if (chassis_path_string_is_parent_of(C(EXEC_PREFIX), C(PLUGINDIR)) {
+		rel_plugin_dir = PLUGINDIR + sizeof(EXEC_PREFIX) - 1;
+
+		if (rel_plugin_dir[0] == G_DIR_SEPARATOR) rel_plugin_dir++; /* if plugindir starts with a /, skip it */
+
+		plugin_dir = g_build_filename(base_dir, rel_plugin_dir, NULL);
+	} else {
+		plugin_dir = g_strdup(PLUGINDIR);
+	}    
+
 #else
 	plugin_dir = g_build_filename(base_dir, "lib", PACKAGE, "plugins", NULL);
 #endif

=== modified file 'src/chassis-path.c'
--- src/chassis-path.c	2010-04-06 14:26:51 +0000
+++ src/chassis-path.c	2012-02-21 13:12:19 +0000
@@ -125,4 +125,44 @@
 	return TRUE;
 }
 
+/**
+ * Check if a path-string is parent of another
+ * @returns TRUE if the path-string is parent, FALSE if don't
+ */
+gboolean chassis_path_string_is_parent_of(const char *parent, gsize parent_len, const char *child, gsize child_len) {
+
+	if (child_len < parent_len)
+		return FALSE;
+
+	if(parent_len == 0)
+		return FALSE;
+
+	/* On Unix systems '/' is parent of all directories' */
+	if(0 == strcmp(parent, "/"))
+		return TRUE;
+
+	/* The path-string child can start with the same prefix string however the directory is not correct,
+	 * e.g. parent: /foo/bar , child: /foo/bar-foo.
+	 * To make the correct comparison we need to check if the parent path-string ends with the directory separator '/',
+	 * and if not, add it before doing the comparison
+	 */
+	if (parent[parent_len - 1] != G_DIR_SEPARATOR) {
+		/* g_strndup adds the trailing '\0' at the end of the new string */
+		char *temp_parent = g_strndup(parent, parent_len + 1);
+		temp_parent[parent_len] = G_DIR_SEPARATOR;
+
+		if (0 == strncmp(temp_parent, child, parent_len + 1)) {
+			g_free(temp_parent);
+			return TRUE;
+		}
+
+		g_free(temp_parent);
+		return FALSE;
+	}
+
+	if (0 == strncmp(parent, child, parent_len))
+		return TRUE;
+
+	return FALSE;
+}
 

=== modified file 'src/chassis-path.h'
--- src/chassis-path.h	2010-04-06 14:26:51 +0000
+++ src/chassis-path.h	2012-02-21 13:12:19 +0000
@@ -27,6 +27,7 @@
 
 CHASSIS_API gboolean chassis_resolve_path(const char *base_dir, gchar **filename);
 CHASSIS_API gchar *chassis_get_basedir(const gchar *prgname);
+CHASSIS_API gboolean chassis_path_string_is_parent_of(const char *parent, gsize parent_len, const char *child, gsize child_len);
 
 #endif
 

=== modified file 'tests/unit/check_chassis_path.c'
--- tests/unit/check_chassis_path.c	2010-04-06 14:26:51 +0000
+++ tests/unit/check_chassis_path.c	2012-02-21 13:12:19 +0000
@@ -28,6 +28,8 @@
 
 #include "chassis-path.h"
 
+#include <string.h>
+
 #if GLIB_CHECK_VERSION(2, 16, 0)
 #define C(x) x, sizeof(x) - 1
 
@@ -94,6 +96,22 @@
 }
 /*@}*/
 
+void test_path_string_is_parent_of(void) {
+	g_assert_cmpint(TRUE, ==, chassis_path_string_is_parent_of(C("/foo"), C("/foo/bar/")));
+	g_assert_cmpint(TRUE, ==, chassis_path_string_is_parent_of(C("/"), C("/foo/")));
+	g_assert_cmpint(TRUE, ==, chassis_path_string_is_parent_of(C("/"), C("/foo")));
+	g_assert_cmpint(TRUE, ==, chassis_path_string_is_parent_of(C("/foo"), C("/foo/")));
+	g_assert_cmpint(TRUE, ==, chassis_path_string_is_parent_of(C("/foo/bar"), C("/foo/bar/bar")));
+	g_assert_cmpint(FALSE, ==, chassis_path_string_is_parent_of(C("/foo/bar"), C("/foo/bar-foo")));
+	g_assert_cmpint(FALSE, ==, chassis_path_string_is_parent_of(C("/foo/bar2"), C("/foo/bar")));
+	g_assert_cmpint(FALSE, ==, chassis_path_string_is_parent_of(C("/foo/bar"), C("/foo/bar2")));
+	g_assert_cmpint(FALSE, ==, chassis_path_string_is_parent_of(C(""), C("/foo")));
+	g_assert_cmpint(FALSE, ==, chassis_path_string_is_parent_of(C("/foo/bar/"), C("/foo/bar2")));
+	g_assert_cmpint(FALSE, ==, chassis_path_string_is_parent_of(C("/foo/bar/"), C("/foo/bar")));
+}
+
+/*@}*/
+
 int main(int argc, char **argv) {
 	g_thread_init(NULL);
 
@@ -103,7 +121,8 @@
 	g_test_add_func("/core/basedir/relpath", test_path_basedir);
 	g_test_add_func("/core/basedir/nobasedir", test_no_basedir);
 	g_test_add_func("/core/basedir/abspath", test_abspath_basedir);
-	
+	g_test_add_func("/core/path_string/is_parent_of", test_path_string_is_parent_of);
+
 	return g_test_run();
 }
 #else