Merge lp:~jamesodhunt/libnih/bug-776532 into lp:~upstart-devel/libnih/nih

[Dimitri John Ledkov]

[Steve Langasek (vorlon)]

The test suite correctly identifies a bug in this proposed branch. The right fix is for *nih_watch_add* to pass the wrapper *to* nih_dir_walk_scan(); nih_dir_walk_scan() should itself not be wrapping anything or assuming anything about the structure of data, which is what's happening here and causing the test suite failure.

I've got a local fix for this which I'll submit as a separate MP shortly.

review: Disapprove

Unmerged revisions

=== modified file 'ChangeLog'
--- ChangeLog	2011-09-01 18:41:03 +0000
+++ ChangeLog	2012-12-17 09:25:26 +0000
@@ -1,3 +1,13 @@
[phone]  James Hunt  <[e-mail]>
+
+	* nih/file.c (nih_dir_walk_filter): New NihFileFilter function passed to
+	nih_dir_walk_scan() to ensure the nih_watch_new() filter function is
+	passed the NihWatch data rather than the data passed to the
+	nih_dir_walk() NihFileVisitor function (LP: #776532).
+
+	* nih/tests/test_watch.c (test_new): New test "with filter and data"
+	to ensure filter is passed correct value.
+
 2011-08-31  James Hunt  <[e-mail]>
 
 	* nih-dbus-tool/tests/test_com.netsplit.Nih.Test_object.c
 
=== modified file 'nih/file.c'
--- nih/file.c	2009-10-03 09:37:58 +0000
+++ nih/file.c	2012-12-17 09:25:26 +0000
@@ -44,6 +44,7 @@
 #include <nih/logging.h>
 #include <nih/error.h>
 #include <nih/errors.h>
+#include <nih/watch.h>
 
 
 /**
@@ -71,6 +72,8 @@
 				  NihFileVisitor visitor,
 				  NihFileErrorHandler error, void *data)
 	__attribute__ ((warn_unused_result));
+static int    nih_dir_walk_filter (void *data, const char *path, int is_dir)
+	__attribute__ ((warn_unused_result));
 
 
 /**
@@ -542,7 +545,7 @@
 	nih_assert (path != NULL);
 	nih_assert (visitor != NULL);
 
-	paths = nih_dir_walk_scan (path, filter, data);
+	paths = nih_dir_walk_scan (path, nih_dir_walk_filter, data);
 	if (! paths)
 		return -1;
 
@@ -571,6 +574,34 @@
 }
 
 /**
+ * nih_dir_walk_filter:
+ * @data: NihWatch,
+ * @path: path to file,
+ * @is_dir: TRUE if @path is a directory.
+ *
+ * NihFileFilter function that is required to ensure the data passed to
+ * the user-specified NihFileFilter (as specified to nih_watch_new())
+ * is the NihWatch @data and not the NihWatch itself.
+ *
+ * Returns: TRUE if the path should be ignored, FALSE otherwise.
+ **/
+static int
+nih_dir_walk_filter (void *data, const char *path, int is_dir)
+{
+    NihWatch *watch;
+
+    watch = (NihWatch *)data;
+
+    nih_assert (watch);
+
+    /* No filter, so accept all files */
+    if (! watch->filter)
+	    return FALSE;
+
+    return watch->filter (watch->data, path, is_dir);
+}
+
+/**
  * nih_dir_walk_sort:
  * @a: pointer to first path,
  * @b: pointer to second path.
 
=== modified file 'nih/tests/test_watch.c'
--- nih/tests/test_watch.c	2011-08-26 18:30:16 +0000
+++ nih/tests/test_watch.c	2012-12-17 09:25:26 +0000
@@ -39,6 +39,8 @@
 #include <nih/error.h>
 #include <nih/logging.h>
 
+/* Read "The Hitchhikers Guide to the Galaxy" */
+#define FILTER_VALUE 42
 
 static int
 my_filter (void       *data,
@@ -54,6 +56,26 @@
 	return FALSE;
 }
 
+/* Set by my_filter2 () so it knows if it has already been called */
+static int my_filter2_called = 0;
+
+static int
+my_filter2 (int       *value,
+	   const char *path,
+	   int         is_dir)
+{
+	/* we only want to toggle the value once */
+	if (my_filter2_called)
+		return TRUE;
+
+	my_filter2_called = 1;
+
+	nih_assert (value && *value == FILTER_VALUE);
+	*value = 0;
+
+	return FALSE;
+}
+
 static int create_called = 0;
 static int modify_called = 0;
 static int delete_called = 0;
@@ -553,6 +575,44 @@
 		nih_free (watch);
 	}
 
+	/* Ensure the file filter gets passed the correct data pointer.
+	 */
+	TEST_FEATURE ("with filter and data");
+
+	/* Ensure we have a new directory */
+	TEST_FILENAME (dirname);
+	mkdir (dirname, 0755);
+
+	/* Create a single file */
+	strcpy (filename, dirname);
+	strcat (filename, "/foo");
+
+	fd = fopen (filename, "w");
+	fprintf (fd, "test\n");
+	fclose (fd);
+
+	TEST_ALLOC_FAIL {
+		int watch_data = FILTER_VALUE;
+
+		/* Reset required to appease TEST_ALLOC_FAIL */
+		my_filter2_called = 0;
+
+		watch = nih_watch_new (NULL, dirname,
+				TRUE, TRUE,
+				(NihFileFilter)my_filter2,
+				NULL, NULL, NULL,
+				&watch_data);
+
+		TEST_NE_P (watch, NULL);
+
+		/* Ensure the filter was called and changed the value */
+
+		TEST_NE (my_filter2_called, 0);
+		TEST_EQ (watch_data, 0);
+
+		nih_free (watch);
+	}
+
 	strcpy (filename, dirname);
 	strcat (filename, "/bar");
 	chmod (filename, 0755);