upstart

Merge lp:~jamesodhunt/upstart/upstart-file-bridge-tidyup into lp:upstart

  1. upstart-file-bridge-tidyup
  2. Merge into trunk
Proposed by James Hunt
Status: Merged
Merged at revision: 1455
Proposed branch: lp:~jamesodhunt/upstart/upstart-file-bridge-tidyup
Merge into: lp:upstart
Diff against target: 236 lines (+46/-41)
2 files modified
ChangeLog (+12/-0)
extra/upstart-file-bridge.c (+34/-41)
To merge this branch: bzr merge lp:~jamesodhunt/upstart/upstart-file-bridge-tidyup
Reviewer Review Type Date Requested Status
Steve Langasek Approve
Review via email: mp+153797@code.launchpad.net

Description of the change

* extra/upstart-file-bridge.c:
  - main(): String safety for home_dir.
  - job_add_file():
    - Initialise events.
    - Use nih_strdup() rather than arrays for paths.
    - Removed unecessary error label.
  - {create_handler,modify_handler,delete_handler}(): Remove strcpy().
  - watched_dir_new(): Use nih_strdup() rather than arrays for path.
  - find_first_parent(): Replace strcpy with strncpy().

To post a comment you must log in.
Revision history for this message
Steve Langasek (vorlon) wrote :

@@ -1683,7 +1675,8 @@

        do {
                /* save parent for next time through the loop */
- strcpy (tmp, current);
+ memset (tmp, '\0', sizeof (tmp));
+ strncpy (tmp, current, sizeof (tmp)-1);
                parent = dirname (tmp);

                /* Ensure dirname returned something sane */

I've always preferred tmp[sizeof (tmp) - 1] = '\0' rather than memset, avoids wasting cycles zero-filling an area that you're just going to overwrite immediately afterwards. But looks good, landing.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ChangeLog'
2--- ChangeLog 2013-03-15 12:51:25 +0000
3+++ ChangeLog 2013-03-18 13:40:33 +0000
4@@ -1,5 +1,17 @@
5 2013-03-15 James Hunt <james.hunt@ubuntu.com>
6
7+ * extra/upstart-file-bridge.c:
8+ - main(): String safety for home_dir.
9+ - job_add_file():
10+ - Initialise events.
11+ - Use nih_strdup() rather than arrays for paths.
12+ - Removed unecessary error label.
13+ - {create_handler,modify_handler,delete_handler}(): Remove strcpy().
14+ - watched_dir_new(): Use nih_strdup() rather than arrays for path.
15+ - find_first_parent(): Replace strcpy with strncpy().
16+
17+2013-03-15 James Hunt <james.hunt@ubuntu.com>
18+
19 * extra/man/file-event.7: Simplify language.
20 * extra/upstart-file-bridge.c:
21 - skip_slashes(): New macro to make path matching more reliable.
22
23=== modified file 'extra/upstart-file-bridge.c'
24--- extra/upstart-file-bridge.c 2013-03-15 12:44:29 +0000
25+++ extra/upstart-file-bridge.c 2013-03-18 13:40:33 +0000
26@@ -451,7 +451,8 @@
27
28 nih_assert (pw->pw_dir);
29
30- strcpy (home_dir, (pw->pw_dir));
31+ memset (home_dir, '\0', sizeof (home_dir));
32+ strncpy (home_dir, pw->pw_dir, sizeof (home_dir)-1);
33 }
34
35 /* Allocate jobs hash table */
36@@ -716,20 +717,18 @@
37 job_add_file (Job *job,
38 char **file_info)
39 {
40- uint32_t events;
41+ uint32_t events = 0x0;
42 WatchedFile *file = NULL;
43 nih_local char *error = NULL;
44 nih_local char *glob_expr = NULL;
45 nih_local char *expanded = NULL;
46- char path[PATH_MAX];
47+ nih_local char *path = NULL;
48
49 nih_assert (job);
50 nih_assert (job->path);
51 nih_assert (file_info);
52 nih_assert (! strcmp (file_info[0], FILE_EVENT));
53
54- memset (path, '\0', sizeof (path));
55-
56 for (char **env = file_info + 1; env && *env; env++) {
57 char *val;
58 size_t name_len;
59@@ -738,33 +737,33 @@
60 if (! val) {
61 nih_warn ("%s: Ignored %s event without variable name",
62 job->path, FILE_EVENT);
63- goto error;
64+ return;
65 }
66
67 name_len = val - *env;
68 val++;
69
70 if (! strncmp (*env, "FPATH", name_len)) {
71- char dirpart[PATH_MAX];
72- char basepart[PATH_MAX];
73+ nih_local char *dirpart = NULL;
74+ nih_local char *basepart = NULL;
75 char *dir;
76 char *base;
77 size_t len2;
78
79- strcpy (path, val);
80+ path = NIH_MUST (nih_strdup (NULL, val));
81
82 if (user && path[0] != '/') {
83 expanded = expand_path (NULL, path);
84 if (! expanded) {
85 nih_error ("Failed to expand path");
86- goto error;
87+ return;
88 }
89 }
90
91 if (! path_valid (path))
92- goto error;
93+ return;
94
95- strcpy (dirpart, path);
96+ dirpart = NIH_MUST (nih_strdup (NULL, path));
97 dir = dirname (dirpart);
98
99 /* See dirname(3) */
100@@ -774,10 +773,10 @@
101
102 if (strcspn (dir, GLOB_CHARS) < len2) {
103 nih_warn ("%s: %s", job->path, _("Directory globbing not supported"));
104- goto error;
105+ return;
106 }
107
108- strcpy (basepart, path);
109+ basepart = NIH_MUST (nih_strdup (NULL, path));
110 base = basename (basepart);
111
112 /* See dirname(3) */
113@@ -786,7 +785,8 @@
114 len2 = strlen (base);
115
116 if (strcspn (base, GLOB_CHARS) < len2) {
117- strcpy (path, dir);
118+ nih_free (path);
119+ path = NIH_MUST (nih_strdup (NULL, dir));
120 glob_expr = NIH_MUST (nih_strdup (NULL, base));
121 }
122 } else if (! strncmp (*env, "FEVENT", name_len)) {
123@@ -800,8 +800,8 @@
124 }
125 }
126
127- if (! *path)
128- goto error;
129+ if (! path)
130+ return;
131
132 if (! events)
133 events = ALL_FILE_EVENTS;
134@@ -813,7 +813,7 @@
135 if (! file) {
136 nih_warn ("%s: %s",
137 _("Failed to add new file"), path);
138- goto error;
139+ return;
140 }
141
142 /* If the job cares about the file or directory existing and it
143@@ -857,12 +857,6 @@
144 }
145
146 ensure_watched (job, file);
147-
148- return;
149-
150-error:
151- if (file)
152- nih_free (file);
153 }
154
155 /**
156@@ -966,12 +960,10 @@
157 nih_list_add (&entries, &file->entry);
158 }
159 } else if (file->glob) {
160- char full_path[PATH_MAX];
161+ nih_local char *full_path = NULL;
162
163 /* reconstruct the full path */
164- strcpy (full_path, file->path);
165- strcat (full_path, "/");
166- strcat (full_path, file->glob);
167+ full_path = NIH_MUST (nih_sprintf (NULL, "%s/%s", file->path, file->glob));
168
169 if (! fnmatch (full_path, path, FNM_PATHNAME) && (file->events & IN_CREATE))
170 handle_event (handled, full_path, IN_CREATE, path);
171@@ -1082,12 +1074,11 @@
172 handle_event (handled, original_path (file), IN_MODIFY, path);
173 }
174 } else if (file->glob) {
175- char full_path[PATH_MAX];
176+ nih_local char *full_path = NULL;
177
178 /* reconstruct the full path */
179- strcpy (full_path, file->path);
180- strcat (full_path, "/");
181- strcat (full_path, file->glob);
182+ full_path = NIH_MUST (nih_sprintf (NULL, "%s/%s", file->path, file->glob));
183+
184 if (! fnmatch (full_path, path, FNM_PATHNAME) && (file->events & IN_MODIFY))
185 handle_event (handled, full_path, IN_MODIFY, path);
186 } else {
187@@ -1157,12 +1148,10 @@
188 handle_event (handled, original_path (file), IN_MODIFY, path);
189 }
190 } else if (file->glob) {
191- char full_path[PATH_MAX];
192+ nih_local char *full_path = NULL;
193
194 /* reconstruct the full path */
195- strcpy (full_path, file->path);
196- strcat (full_path, "/");
197- strcat (full_path, file->glob);
198+ full_path = NIH_MUST (nih_sprintf (NULL, "%s/%s", file->path, file->glob));
199
200 if (! fnmatch (full_path, path, FNM_PATHNAME) && (file->events & IN_DELETE))
201 handle_event (handled, full_path, IN_DELETE, path);
202@@ -1423,9 +1412,9 @@
203 watched_dir_new (const char *path,
204 const struct stat *statbuf)
205 {
206- char watched_path[PATH_MAX];
207- size_t len;
208- WatchedDir *dir;
209+ nih_local char *watched_path = NULL;
210+ WatchedDir *dir;
211+ size_t len;
212
213 nih_assert (path);
214 nih_assert (statbuf);
215@@ -1435,7 +1424,10 @@
216
217 watched_dir_init ();
218
219- strcpy (watched_path, path);
220+ watched_path = nih_strdup (NULL, path);
221+ if (! watched_path)
222+ return NULL;
223+
224 len = strlen (watched_path);
225
226 if (len > 1 && watched_path[len-1] == '/') {
227@@ -1683,7 +1675,8 @@
228
229 do {
230 /* save parent for next time through the loop */
231- strcpy (tmp, current);
232+ memset (tmp, '\0', sizeof (tmp));
233+ strncpy (tmp, current, sizeof (tmp)-1);
234 parent = dirname (tmp);
235
236 /* Ensure dirname returned something sane */

Subscribers

People subscribed via source and target branches