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

Proposed by Dimitri John Ledkov on 2012-12-17
Status: Rejected
Rejected by: Steve Langasek on 2013-03-13
Proposed branch: lp:~jamesodhunt/libnih/bug-776532
Merge into: lp:~upstart-devel/libnih/nih
Diff against target: 167 lines (+102/-1)
3 files modified
ChangeLog (+10/-0)
nih/file.c (+32/-1)
nih/tests/test_watch.c (+60/-0)
To merge this branch: bzr merge lp:~jamesodhunt/libnih/bug-776532
Reviewer Review Type Date Requested Status
Steve Langasek 2012-12-17 Disapprove on 2013-03-13
Review via email: mp+140150@code.launchpad.net
To post a comment you must log in.
Steve Langasek (vorlon) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ChangeLog'
2--- ChangeLog 2011-09-01 18:41:03 +0000
3+++ ChangeLog 2012-12-17 09:25:26 +0000
4@@ -1,3 +1,13 @@
5+2012-04-27 James Hunt <james.hunt@ubuntu.com>
6+
7+ * nih/file.c (nih_dir_walk_filter): New NihFileFilter function passed to
8+ nih_dir_walk_scan() to ensure the nih_watch_new() filter function is
9+ passed the NihWatch data rather than the data passed to the
10+ nih_dir_walk() NihFileVisitor function (LP: #776532).
11+
12+ * nih/tests/test_watch.c (test_new): New test "with filter and data"
13+ to ensure filter is passed correct value.
14+
15 2011-08-31 James Hunt <james.hunt@ubuntu.com>
16
17 * nih-dbus-tool/tests/test_com.netsplit.Nih.Test_object.c
18
19=== modified file 'nih/file.c'
20--- nih/file.c 2009-10-03 09:37:58 +0000
21+++ nih/file.c 2012-12-17 09:25:26 +0000
22@@ -44,6 +44,7 @@
23 #include <nih/logging.h>
24 #include <nih/error.h>
25 #include <nih/errors.h>
26+#include <nih/watch.h>
27
28
29 /**
30@@ -71,6 +72,8 @@
31 NihFileVisitor visitor,
32 NihFileErrorHandler error, void *data)
33 __attribute__ ((warn_unused_result));
34+static int nih_dir_walk_filter (void *data, const char *path, int is_dir)
35+ __attribute__ ((warn_unused_result));
36
37
38 /**
39@@ -542,7 +545,7 @@
40 nih_assert (path != NULL);
41 nih_assert (visitor != NULL);
42
43- paths = nih_dir_walk_scan (path, filter, data);
44+ paths = nih_dir_walk_scan (path, nih_dir_walk_filter, data);
45 if (! paths)
46 return -1;
47
48@@ -571,6 +574,34 @@
49 }
50
51 /**
52+ * nih_dir_walk_filter:
53+ * @data: NihWatch,
54+ * @path: path to file,
55+ * @is_dir: TRUE if @path is a directory.
56+ *
57+ * NihFileFilter function that is required to ensure the data passed to
58+ * the user-specified NihFileFilter (as specified to nih_watch_new())
59+ * is the NihWatch @data and not the NihWatch itself.
60+ *
61+ * Returns: TRUE if the path should be ignored, FALSE otherwise.
62+ **/
63+static int
64+nih_dir_walk_filter (void *data, const char *path, int is_dir)
65+{
66+ NihWatch *watch;
67+
68+ watch = (NihWatch *)data;
69+
70+ nih_assert (watch);
71+
72+ /* No filter, so accept all files */
73+ if (! watch->filter)
74+ return FALSE;
75+
76+ return watch->filter (watch->data, path, is_dir);
77+}
78+
79+/**
80 * nih_dir_walk_sort:
81 * @a: pointer to first path,
82 * @b: pointer to second path.
83
84=== modified file 'nih/tests/test_watch.c'
85--- nih/tests/test_watch.c 2011-08-26 18:30:16 +0000
86+++ nih/tests/test_watch.c 2012-12-17 09:25:26 +0000
87@@ -39,6 +39,8 @@
88 #include <nih/error.h>
89 #include <nih/logging.h>
90
91+/* Read "The Hitchhikers Guide to the Galaxy" */
92+#define FILTER_VALUE 42
93
94 static int
95 my_filter (void *data,
96@@ -54,6 +56,26 @@
97 return FALSE;
98 }
99
100+/* Set by my_filter2 () so it knows if it has already been called */
101+static int my_filter2_called = 0;
102+
103+static int
104+my_filter2 (int *value,
105+ const char *path,
106+ int is_dir)
107+{
108+ /* we only want to toggle the value once */
109+ if (my_filter2_called)
110+ return TRUE;
111+
112+ my_filter2_called = 1;
113+
114+ nih_assert (value && *value == FILTER_VALUE);
115+ *value = 0;
116+
117+ return FALSE;
118+}
119+
120 static int create_called = 0;
121 static int modify_called = 0;
122 static int delete_called = 0;
123@@ -553,6 +575,44 @@
124 nih_free (watch);
125 }
126
127+ /* Ensure the file filter gets passed the correct data pointer.
128+ */
129+ TEST_FEATURE ("with filter and data");
130+
131+ /* Ensure we have a new directory */
132+ TEST_FILENAME (dirname);
133+ mkdir (dirname, 0755);
134+
135+ /* Create a single file */
136+ strcpy (filename, dirname);
137+ strcat (filename, "/foo");
138+
139+ fd = fopen (filename, "w");
140+ fprintf (fd, "test\n");
141+ fclose (fd);
142+
143+ TEST_ALLOC_FAIL {
144+ int watch_data = FILTER_VALUE;
145+
146+ /* Reset required to appease TEST_ALLOC_FAIL */
147+ my_filter2_called = 0;
148+
149+ watch = nih_watch_new (NULL, dirname,
150+ TRUE, TRUE,
151+ (NihFileFilter)my_filter2,
152+ NULL, NULL, NULL,
153+ &watch_data);
154+
155+ TEST_NE_P (watch, NULL);
156+
157+ /* Ensure the filter was called and changed the value */
158+
159+ TEST_NE (my_filter2_called, 0);
160+ TEST_EQ (watch_data, 0);
161+
162+ nih_free (watch);
163+ }
164+
165 strcpy (filename, dirname);
166 strcat (filename, "/bar");
167 chmod (filename, 0755);

Subscribers

People subscribed via source and target branches

to all changes: