Ticket #4282: ticket4282.patch.v2

File ticket4282.patch.v2, 25.7 KB (added by piet, 7 years ago)
Line 
1From c8455da2f5c8797f3d51996e5b7a83de681d30dc Mon Sep 17 00:00:00 2001
2From: Peter Retzlaff <pe.retzlaff@gmail.com>
3Date: Fri, 25 Jan 2013 11:49:33 +0100
4Subject: [PATCH 1/7] Extract duplicate code in geoip and rephist.
5
6Create new methods check_or_create_data_subdir() and
7write_to_data_subdir() in config.c and use them throughout
8rephist.c and geoip.c.
9This should solve ticket #4282.
10---
11 src/or/config.c  |   35 ++++++++++++++++++++++++++++
12 src/or/config.h  |    4 ++++
13 src/or/geoip.c   |   67 +++++++++++++++++++-----------------------------------
14 src/or/rephist.c |   48 ++++++++++----------------------------
15 4 files changed, 75 insertions(+), 79 deletions(-)
16
17diff --git a/src/or/config.c b/src/or/config.c
18index 31695ba..489b369 100644
19--- a/src/or/config.c
20+++ b/src/or/config.c
21@@ -5786,6 +5786,41 @@ options_get_datadir_fname2_suffix(const or_options_t *options,
22   return fname;
23 }
24 
25+/** Check wether the data directory has a private subdirectory
26+ * <b>subdir</b>. If not, try to create it. Return 0 on success,
27+ * -1 otherwise. */
28+int
29+check_or_create_data_subdir(const char *subdir) {
30+  char *statsdir = get_datadir_fname(subdir);
31+  int return_val = 0;
32+
33+  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
34+    log_warn(LD_HIST, "Unable to create %s/ directory!", subdir);
35+       return_val = -1;
36+  }
37+  tor_free(statsdir);
38+  return return_val;
39+}
40+
41+/** Create a file named <b>fname</b> with contents <b>str</b> in the
42+ * subdirectory <b>subdir</b> of the data directory. <b>descr</b>
43+ * should be a short description of the file's content and will be
44+ * used for the warning message, if it's present and the write process
45+ * fails. Return 0 on success, -1 otherwise.*/
46+int
47+write_to_data_subdir(const char* subdir, const char* fname,
48+                     const char* str, const char* descr) {
49+  char *filename = get_datadir_fname2(subdir, fname);
50+  int return_val = 0;
51+
52+  if (write_str_to_file(filename, str, 0) < 0) {
53+    log_warn(LD_HIST, "Unable to write %s to disk!", descr ? descr : fname);
54+    return_val = -1;
55+  }
56+  tor_free(filename);
57+  return return_val;
58+}
59+
60 /** Given a file name check to see whether the file exists but has not been
61  * modified for a very long time.  If so, remove it. */
62 void
63diff --git a/src/or/config.h b/src/or/config.h
64index 8e34655..d0ffda7 100644
65--- a/src/or/config.h
66+++ b/src/or/config.h
67@@ -57,6 +57,10 @@ char *options_get_datadir_fname2_suffix(const or_options_t *options,
68 #define get_datadir_fname_suffix(sub1, suffix) \
69   get_datadir_fname2_suffix((sub1), NULL, (suffix))
70 
71+int check_or_create_data_subdir(const char *subdir);
72+int write_to_data_subdir(const char* subdir, const char* fname,
73+                                const char* str, const char* descr);
74+
75 int get_num_cpus(const or_options_t *options);
76 
77 const smartlist_t *get_configured_ports(void);
78diff --git a/src/or/geoip.c b/src/or/geoip.c
79index 9ba1e31..b692b3a 100644
80--- a/src/or/geoip.c
81+++ b/src/or/geoip.c
82@@ -1132,7 +1132,7 @@ geoip_format_dirreq_stats(time_t now)
83 time_t
84 geoip_dirreq_stats_write(time_t now)
85 {
86-  char *statsdir = NULL, *filename = NULL, *str = NULL;
87+  char *str = NULL;
88 
89   if (!start_of_dirreq_stats_interval)
90     return 0; /* Not initialized. */
91@@ -1146,21 +1146,13 @@ geoip_dirreq_stats_write(time_t now)
92   str = geoip_format_dirreq_stats(now);
93 
94   /* Write dirreq-stats string to disk. */
95-  statsdir = get_datadir_fname("stats");
96-  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
97-    log_warn(LD_HIST, "Unable to create stats/ directory!");
98-    goto done;
99+  if (check_or_create_data_subdir("stats") < 0) {
100+       write_to_data_subdir("stats", "dirreq-stats", str, "dirreq statistics");
101+       /* Reset measurement interval start. */
102+       geoip_reset_dirreq_stats(now);
103   }
104-  filename = get_datadir_fname2("stats", "dirreq-stats");
105-  if (write_str_to_file(filename, str, 0) < 0)
106-    log_warn(LD_HIST, "Unable to write dirreq statistics to disk!");
107-
108-  /* Reset measurement interval start. */
109-  geoip_reset_dirreq_stats(now);
110 
111  done:
112-  tor_free(statsdir);
113-  tor_free(filename);
114   tor_free(str);
115   return start_of_dirreq_stats_interval + WRITE_STATS_INTERVAL;
116 }
117@@ -1297,7 +1289,7 @@ format_bridge_stats_controller(time_t now)
118 time_t
119 geoip_bridge_stats_write(time_t now)
120 {
121-  char *filename = NULL, *val = NULL, *statsdir = NULL;
122+  char *val = NULL;
123 
124   /* Check if 24 hours have passed since starting measurements. */
125   if (now < start_of_bridge_stats_interval + WRITE_STATS_INTERVAL)
126@@ -1317,24 +1309,20 @@ geoip_bridge_stats_write(time_t now)
127   start_of_bridge_stats_interval = now;
128 
129   /* Write it to disk. */
130-  statsdir = get_datadir_fname("stats");
131-  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
132-    goto done;
133-  filename = get_datadir_fname2("stats", "bridge-stats");
134-
135-  write_str_to_file(filename, bridge_stats_extrainfo, 0);
136-
137-  /* Tell the controller, "hey, there are clients!" */
138-  {
139-    char *controller_str = format_bridge_stats_controller(now);
140-    if (controller_str)
141-      control_event_clients_seen(controller_str);
142-    tor_free(controller_str);
143+  if (!check_or_create_data_subdir("stats")) {
144+       write_to_data_subdir("stats", "bridge-stats",
145+                       bridge_stats_extrainfo, "bridge statistics");
146+
147+       /* Tell the controller, "hey, there are clients!" */
148+       {
149+         char *controller_str = format_bridge_stats_controller(now);
150+         if (controller_str)
151+           control_event_clients_seen(controller_str);
152+         tor_free(controller_str);
153+       }
154   }
155- done:
156-  tor_free(filename);
157-  tor_free(statsdir);
158 
159+ done:
160   return start_of_bridge_stats_interval + WRITE_STATS_INTERVAL;
161 }
162 
163@@ -1433,7 +1421,7 @@ geoip_format_entry_stats(time_t now)
164 time_t
165 geoip_entry_stats_write(time_t now)
166 {
167-  char *statsdir = NULL, *filename = NULL, *str = NULL;
168+  char *str = NULL;
169 
170   if (!start_of_entry_stats_interval)
171     return 0; /* Not initialized. */
172@@ -1447,21 +1435,14 @@ geoip_entry_stats_write(time_t now)
173   str = geoip_format_entry_stats(now);
174 
175   /* Write entry-stats string to disk. */
176-  statsdir = get_datadir_fname("stats");
177-  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
178-    log_warn(LD_HIST, "Unable to create stats/ directory!");
179-    goto done;
180-  }
181-  filename = get_datadir_fname2("stats", "entry-stats");
182-  if (write_str_to_file(filename, str, 0) < 0)
183-    log_warn(LD_HIST, "Unable to write entry statistics to disk!");
184+  if (!check_or_create_data_subdir("stats")) {
185+         write_to_data_subdir("stats", "entry-stats", str, "entry statistics");
186 
187-  /* Reset measurement interval start. */
188-  geoip_reset_entry_stats(now);
189+         /* Reset measurement interval start. */
190+         geoip_reset_entry_stats(now);
191+  }
192 
193  done:
194-  tor_free(statsdir);
195-  tor_free(filename);
196   tor_free(str);
197   return start_of_entry_stats_interval + WRITE_STATS_INTERVAL;
198 }
199diff --git a/src/or/rephist.c b/src/or/rephist.c
200index 34caa4b..0a5d420 100644
201--- a/src/or/rephist.c
202+++ b/src/or/rephist.c
203@@ -2312,7 +2312,7 @@ rep_hist_format_exit_stats(time_t now)
204 time_t
205 rep_hist_exit_stats_write(time_t now)
206 {
207-  char *statsdir = NULL, *filename = NULL, *str = NULL;
208+  char *str = NULL;
209 
210   if (!start_of_exit_stats_interval)
211     return 0; /* Not initialized. */
212@@ -2328,19 +2328,12 @@ rep_hist_exit_stats_write(time_t now)
213   rep_hist_reset_exit_stats(now);
214 
215   /* Try to write to disk. */
216-  statsdir = get_datadir_fname("stats");
217-  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
218-    log_warn(LD_HIST, "Unable to create stats/ directory!");
219-    goto done;
220+  if (!check_or_create_data_subdir("stats")) {
221+    write_to_data_subdir("stats", "exit-stats", str, "exit port statistics");
222   }
223-  filename = get_datadir_fname2("stats", "exit-stats");
224-  if (write_str_to_file(filename, str, 0) < 0)
225-    log_warn(LD_HIST, "Unable to write exit port statistics to disk!");
226 
227  done:
228   tor_free(str);
229-  tor_free(statsdir);
230-  tor_free(filename);
231   return start_of_exit_stats_interval + WRITE_STATS_INTERVAL;
232 }
233 
234@@ -2597,7 +2590,7 @@ time_t
235 rep_hist_buffer_stats_write(time_t now)
236 {
237   circuit_t *circ;
238-  char *statsdir = NULL, *filename = NULL, *str = NULL;
239+  char *str = NULL;
240 
241   if (!start_of_buffer_stats_interval)
242     return 0; /* Not initialized. */
243@@ -2616,19 +2609,12 @@ rep_hist_buffer_stats_write(time_t now)
244   rep_hist_reset_buffer_stats(now);
245 
246   /* Try to write to disk. */
247-  statsdir = get_datadir_fname("stats");
248-  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
249-    log_warn(LD_HIST, "Unable to create stats/ directory!");
250-    goto done;
251+  if (!check_or_create_data_subdir("stats")) {
252+    write_to_data_subdir("stats", "buffer-stats", str, "buffer statistics");
253   }
254-  filename = get_datadir_fname2("stats", "buffer-stats");
255-  if (write_str_to_file(filename, str, 0) < 0)
256-    log_warn(LD_HIST, "Unable to write buffer stats to disk!");
257 
258  done:
259   tor_free(str);
260-  tor_free(filename);
261-  tor_free(statsdir);
262   return start_of_buffer_stats_interval + WRITE_STATS_INTERVAL;
263 }
264 
265@@ -2740,7 +2726,7 @@ rep_hist_format_desc_stats(time_t now)
266 time_t
267 rep_hist_desc_stats_write(time_t now)
268 {
269-  char *statsdir = NULL, *filename = NULL, *str = NULL;
270+  char *filename = NULL, *str = NULL;
271 
272   if (!start_of_served_descs_stats_interval)
273     return 0; /* We're not collecting stats. */
274@@ -2750,10 +2736,8 @@ rep_hist_desc_stats_write(time_t now)
275   str = rep_hist_format_desc_stats(now);
276   tor_assert(str != NULL);
277 
278-  statsdir = get_datadir_fname("stats");
279-  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
280-    log_warn(LD_HIST, "Unable to create stats/ directory!");
281-      goto done;
282+  if (check_or_create_data_subdir("stats") < 0) {
283+    goto done;
284   }
285   filename = get_datadir_fname2("stats", "served-desc-stats");
286   if (append_bytes_to_file(filename, str, strlen(str), 0) < 0)
287@@ -2762,7 +2746,6 @@ rep_hist_desc_stats_write(time_t now)
288   rep_hist_reset_desc_stats(now);
289 
290  done:
291-  tor_free(statsdir);
292   tor_free(filename);
293   tor_free(str);
294   return start_of_served_descs_stats_interval + WRITE_STATS_INTERVAL;
295@@ -2980,7 +2963,7 @@ rep_hist_format_conn_stats(time_t now)
296 time_t
297 rep_hist_conn_stats_write(time_t now)
298 {
299-  char *statsdir = NULL, *filename = NULL, *str = NULL;
300+  char *str = NULL;
301 
302   if (!start_of_conn_stats_interval)
303     return 0; /* Not initialized. */
304@@ -2994,19 +2977,12 @@ rep_hist_conn_stats_write(time_t now)
305   rep_hist_reset_conn_stats(now);
306 
307   /* Try to write to disk. */
308-  statsdir = get_datadir_fname("stats");
309-  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
310-    log_warn(LD_HIST, "Unable to create stats/ directory!");
311-    goto done;
312+  if (!check_or_create_data_subdir("stats")) {
313+    write_to_data_subdir("stats", "conn-stats", str, "connection statistics");
314   }
315-  filename = get_datadir_fname2("stats", "conn-stats");
316-  if (write_str_to_file(filename, str, 0) < 0)
317-    log_warn(LD_HIST, "Unable to write conn stats to disk!");
318 
319  done:
320   tor_free(str);
321-  tor_free(filename);
322-  tor_free(statsdir);
323   return start_of_conn_stats_interval + WRITE_STATS_INTERVAL;
324 }
325 
326--
3271.7.9.5
328
329
330From 13be21fe77988ec74b4e5c4934dc73d06fca692a Mon Sep 17 00:00:00 2001
331From: Peter Retzlaff <pe.retzlaff@gmail.com>
332Date: Fri, 1 Feb 2013 20:08:15 +0100
333Subject: [PATCH 2/7] Replaced tabs with spaces.
334
335---
336 src/or/config.c |    2 +-
337 src/or/config.h |    2 +-
338 src/or/geoip.c  |   32 ++++++++++++++++----------------
339 3 files changed, 18 insertions(+), 18 deletions(-)
340
341diff --git a/src/or/config.c b/src/or/config.c
342index 489b369..e726715 100644
343--- a/src/or/config.c
344+++ b/src/or/config.c
345@@ -5796,7 +5796,7 @@ check_or_create_data_subdir(const char *subdir) {
346 
347   if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
348     log_warn(LD_HIST, "Unable to create %s/ directory!", subdir);
349-       return_val = -1;
350+    return_val = -1;
351   }
352   tor_free(statsdir);
353   return return_val;
354diff --git a/src/or/config.h b/src/or/config.h
355index d0ffda7..3dd2315 100644
356--- a/src/or/config.h
357+++ b/src/or/config.h
358@@ -59,7 +59,7 @@ char *options_get_datadir_fname2_suffix(const or_options_t *options,
359 
360 int check_or_create_data_subdir(const char *subdir);
361 int write_to_data_subdir(const char* subdir, const char* fname,
362-                                const char* str, const char* descr);
363+                         const char* str, const char* descr);
364 
365 int get_num_cpus(const or_options_t *options);
366 
367diff --git a/src/or/geoip.c b/src/or/geoip.c
368index b692b3a..9ae41ea 100644
369--- a/src/or/geoip.c
370+++ b/src/or/geoip.c
371@@ -1147,9 +1147,9 @@ geoip_dirreq_stats_write(time_t now)
372 
373   /* Write dirreq-stats string to disk. */
374   if (check_or_create_data_subdir("stats") < 0) {
375-       write_to_data_subdir("stats", "dirreq-stats", str, "dirreq statistics");
376-       /* Reset measurement interval start. */
377-       geoip_reset_dirreq_stats(now);
378+    write_to_data_subdir("stats", "dirreq-stats", str, "dirreq statistics");
379+    /* Reset measurement interval start. */
380+    geoip_reset_dirreq_stats(now);
381   }
382 
383  done:
384@@ -1310,16 +1310,16 @@ geoip_bridge_stats_write(time_t now)
385 
386   /* Write it to disk. */
387   if (!check_or_create_data_subdir("stats")) {
388-       write_to_data_subdir("stats", "bridge-stats",
389-                       bridge_stats_extrainfo, "bridge statistics");
390-
391-       /* Tell the controller, "hey, there are clients!" */
392-       {
393-         char *controller_str = format_bridge_stats_controller(now);
394-         if (controller_str)
395-           control_event_clients_seen(controller_str);
396-         tor_free(controller_str);
397-       }
398+    write_to_data_subdir("stats", "bridge-stats",
399+            bridge_stats_extrainfo, "bridge statistics");
400+
401+    /* Tell the controller, "hey, there are clients!" */
402+    {
403+      char *controller_str = format_bridge_stats_controller(now);
404+      if (controller_str)
405+        control_event_clients_seen(controller_str);
406+      tor_free(controller_str);
407+    }
408   }
409 
410  done:
411@@ -1436,10 +1436,10 @@ geoip_entry_stats_write(time_t now)
412 
413   /* Write entry-stats string to disk. */
414   if (!check_or_create_data_subdir("stats")) {
415-         write_to_data_subdir("stats", "entry-stats", str, "entry statistics");
416+      write_to_data_subdir("stats", "entry-stats", str, "entry statistics");
417 
418-         /* Reset measurement interval start. */
419-         geoip_reset_entry_stats(now);
420+      /* Reset measurement interval start. */
421+      geoip_reset_entry_stats(now);
422   }
423 
424  done:
425--
4261.7.9.5
427
428
429From 53e64d65d91eaf387c94eb37f36e9a3f3b84bac0 Mon Sep 17 00:00:00 2001
430From: Peter Retzlaff <pe.retzlaff@gmail.com>
431Date: Fri, 1 Feb 2013 20:15:23 +0100
432Subject: [PATCH 3/7] Fixed braces for function definitions.
433
434---
435 src/or/config.c |    6 ++++--
436 1 file changed, 4 insertions(+), 2 deletions(-)
437
438diff --git a/src/or/config.c b/src/or/config.c
439index e726715..04461c1 100644
440--- a/src/or/config.c
441+++ b/src/or/config.c
442@@ -5790,7 +5790,8 @@ options_get_datadir_fname2_suffix(const or_options_t *options,
443  * <b>subdir</b>. If not, try to create it. Return 0 on success,
444  * -1 otherwise. */
445 int
446-check_or_create_data_subdir(const char *subdir) {
447+check_or_create_data_subdir(const char *subdir)
448+{
449   char *statsdir = get_datadir_fname(subdir);
450   int return_val = 0;
451 
452@@ -5809,7 +5810,8 @@ check_or_create_data_subdir(const char *subdir) {
453  * fails. Return 0 on success, -1 otherwise.*/
454 int
455 write_to_data_subdir(const char* subdir, const char* fname,
456-                     const char* str, const char* descr) {
457+                     const char* str, const char* descr)
458+{
459   char *filename = get_datadir_fname2(subdir, fname);
460   int return_val = 0;
461 
462--
4631.7.9.5
464
465
466From 717c51b9e1c4506e95e34efb9a904a8a179dd908 Mon Sep 17 00:00:00 2001
467From: Peter Retzlaff <pe.retzlaff@gmail.com>
468Date: Sat, 2 Feb 2013 01:40:41 +0100
469Subject: [PATCH 4/7] Unit tests for check_or_create_data_subdir and
470 write_to_data_subdir.
471
472---
473 src/test/test_config.c |   91 ++++++++++++++++++++++++++++++++++++++++++++++++
474 1 file changed, 91 insertions(+)
475
476diff --git a/src/test/test_config.c b/src/test/test_config.c
477index e20fe73..979b9b5 100644
478--- a/src/test/test_config.c
479+++ b/src/test/test_config.c
480@@ -169,11 +169,102 @@ test_config_addressmap(void *arg)
481   ;
482 }
483 
484+static int
485+is_private_dir(const char* path)
486+{
487+  struct stat st;
488+  int r = stat(path, &st);
489+  if (r) {
490+    return 0;
491+  }
492+#if !defined (_WIN32) || defined (WINCE)
493+  if (st.st_mode != (S_IFDIR | 0700)) {
494+    return 0;
495+  }
496+#endif
497+  return 1;
498+}
499+
500+static void
501+test_config_check_or_create_data_subdir(void *arg)
502+{
503+  or_options_t* options = get_options_mutable();
504+  options->DataDirectory = "test_data";
505+  const char* subdir = "test_stats";
506+  const char* subpath = get_datadir_fname(subdir);
507+  struct stat st;
508+
509+#if defined (_WIN32) && !defined (WINCE)
510+  mkdir(options->DataDirectory);
511+#else
512+  mkdir(options->DataDirectory, 0700);
513+#endif
514+
515+  int r = stat(subpath, &st);
516+
517+  // subdirectory shouldn't exist yet
518+  test_assert(r && errno == ENOENT);
519+  test_assert(!check_or_create_data_subdir(subdir));
520+
521+  test_assert(is_private_dir(subpath));
522+  test_assert(!check_or_create_data_subdir(subdir));
523+
524+  unsigned group_permission = st.st_mode | 0070;
525+  r = chmod(subpath, group_permission);
526+
527+  if (r) {
528+    test_fail_msg("Changing permissions for the subdirectory failed.");
529+  }
530+
531+  test_assert(!is_private_dir(subpath));
532+  test_assert(!check_or_create_data_subdir(subdir));
533+  test_assert(is_private_dir(subpath));
534+
535+  done:
536+    rmdir(subpath);
537+    rmdir(options->DataDirectory);
538+}
539+
540+static void
541+test_config_write_to_data_subdir(void* arg)
542+{
543+  or_options_t* options = get_options_mutable();
544+  options->DataDirectory = "test_data";
545+  const char* subdir = "test_stats";
546+  const char* fname = "test_file";
547+  const char* str = "TEST STRING!!";
548+  const char* subpath = get_datadir_fname(subdir);
549+  const char* filepath = get_datadir_fname2(subdir, fname);
550+
551+#if defined (_WIN32) && !defined (WINCE)
552+  mkdir(options->DataDirectory);
553+#else
554+  mkdir(options->DataDirectory, 0700);
555+#endif
556+
557+  test_assert(write_to_data_subdir(subdir, fname, str, NULL));
558+  check_or_create_data_subdir(subdir);
559+
560+  test_assert(!write_to_data_subdir(subdir, fname, str, NULL));
561+  test_streq(read_file_to_str(filepath, 0, NULL), str);
562+
563+  // do it a second time, to assure that old content is overwritten
564+  test_assert(!write_to_data_subdir(subdir, fname, str, NULL));
565+  test_streq(read_file_to_str(filepath, 0, NULL), str);
566+
567+ done:
568+  remove(filepath);
569+  rmdir(subpath);
570+  rmdir(options->DataDirectory);
571+}
572+
573 #define CONFIG_TEST(name, flags)                          \
574   { #name, test_config_ ## name, flags, NULL, NULL }
575 
576 struct testcase_t config_tests[] = {
577   CONFIG_TEST(addressmap, 0),
578+  CONFIG_TEST(check_or_create_data_subdir, 0),
579+  CONFIG_TEST(write_to_data_subdir, 0),
580   END_OF_TESTCASES
581 };
582 
583--
5841.7.9.5
585
586
587From 84fd617be126d50d2913a4795caa15ee174d997c Mon Sep 17 00:00:00 2001
588From: Peter Retzlaff <pe.retzlaff@gmail.com>
589Date: Sat, 2 Feb 2013 02:19:09 +0100
590Subject: [PATCH 5/7] Comment unit tests and use longer test string for
591 writing.
592
593---
594 src/test/test_config.c |   22 ++++++++++++++++++----
595 1 file changed, 18 insertions(+), 4 deletions(-)
596
597diff --git a/src/test/test_config.c b/src/test/test_config.c
598index 979b9b5..3b6bdad 100644
599--- a/src/test/test_config.c
600+++ b/src/test/test_config.c
601@@ -202,11 +202,14 @@ test_config_check_or_create_data_subdir(void *arg)
602 
603   int r = stat(subpath, &st);
604 
605-  // subdirectory shouldn't exist yet
606+  // The subdirectory shouldn't exist yet,
607+  // but should be created by the call to check_or_create_data_subdir.
608   test_assert(r && errno == ENOENT);
609   test_assert(!check_or_create_data_subdir(subdir));
610-
611   test_assert(is_private_dir(subpath));
612+
613+  // The check should return 0, if the directory already exists
614+  // and is private to the user.
615   test_assert(!check_or_create_data_subdir(subdir));
616 
617   unsigned group_permission = st.st_mode | 0070;
618@@ -216,6 +219,8 @@ test_config_check_or_create_data_subdir(void *arg)
619     test_fail_msg("Changing permissions for the subdirectory failed.");
620   }
621 
622+  // If the directory exists, but its mode is too permissive
623+  // a call to check_or_create_data_subdir should reset the mode.
624   test_assert(!is_private_dir(subpath));
625   test_assert(!check_or_create_data_subdir(subdir));
626   test_assert(is_private_dir(subpath));
627@@ -232,7 +237,14 @@ test_config_write_to_data_subdir(void* arg)
628   options->DataDirectory = "test_data";
629   const char* subdir = "test_stats";
630   const char* fname = "test_file";
631-  const char* str = "TEST STRING!!";
632+  const char* str =
633+      "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod\n"
634+      "tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua.\n"
635+      "At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren,\n"
636+      "no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet,\n"
637+      "consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore\n"
638+      "magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et\n"
639+      "ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.";
640   const char* subpath = get_datadir_fname(subdir);
641   const char* filepath = get_datadir_fname2(subdir, fname);
642 
643@@ -242,13 +254,15 @@ test_config_write_to_data_subdir(void* arg)
644   mkdir(options->DataDirectory, 0700);
645 #endif
646 
647+  // Write attempt shoudl fail, if subdirectory doesn't exist.
648   test_assert(write_to_data_subdir(subdir, fname, str, NULL));
649   check_or_create_data_subdir(subdir);
650 
651+  // Content of file after write attempt should be equal to the original string.
652   test_assert(!write_to_data_subdir(subdir, fname, str, NULL));
653   test_streq(read_file_to_str(filepath, 0, NULL), str);
654 
655-  // do it a second time, to assure that old content is overwritten
656+  // A second write operation should overwrite the old content.
657   test_assert(!write_to_data_subdir(subdir, fname, str, NULL));
658   test_streq(read_file_to_str(filepath, 0, NULL), str);
659 
660--
6611.7.9.5
662
663
664From 7620f8585c4a46fe882c2d7a54cc11042cecf498 Mon Sep 17 00:00:00 2001
665From: Peter Retzlaff <pe.retzlaff@gmail.com>
666Date: Sat, 2 Feb 2013 03:11:15 +0100
667Subject: [PATCH 6/7] Fix failing test case in
668 test_config_check_or_create_data_subdir for Windows.
669
670---
671 src/test/test_config.c |    5 +++--
672 1 file changed, 3 insertions(+), 2 deletions(-)
673
674diff --git a/src/test/test_config.c b/src/test/test_config.c
675index 3b6bdad..4562baf 100644
676--- a/src/test/test_config.c
677+++ b/src/test/test_config.c
678@@ -204,7 +204,7 @@ test_config_check_or_create_data_subdir(void *arg)
679 
680   // The subdirectory shouldn't exist yet,
681   // but should be created by the call to check_or_create_data_subdir.
682-  test_assert(r && errno == ENOENT);
683+  test_assert(r && (errno == ENOENT));
684   test_assert(!check_or_create_data_subdir(subdir));
685   test_assert(is_private_dir(subpath));
686 
687@@ -212,6 +212,7 @@ test_config_check_or_create_data_subdir(void *arg)
688   // and is private to the user.
689   test_assert(!check_or_create_data_subdir(subdir));
690 
691+#if !defined (_WIN32) || defined (WINCE)
692   unsigned group_permission = st.st_mode | 0070;
693   r = chmod(subpath, group_permission);
694 
695@@ -224,6 +225,7 @@ test_config_check_or_create_data_subdir(void *arg)
696   test_assert(!is_private_dir(subpath));
697   test_assert(!check_or_create_data_subdir(subdir));
698   test_assert(is_private_dir(subpath));
699+#endif
700 
701   done:
702     rmdir(subpath);
703@@ -281,4 +283,3 @@ struct testcase_t config_tests[] = {
704   CONFIG_TEST(write_to_data_subdir, 0),
705   END_OF_TESTCASES
706 };
707-
708--
7091.7.9.5
710
711
712From cf14d72c7d11e9f81260591c112141ed88732aba Mon Sep 17 00:00:00 2001
713From: Peter Retzlaff <pe.retzlaff@gmail.com>
714Date: Sat, 2 Feb 2013 03:17:32 +0100
715Subject: [PATCH 7/7] Breaking long lines.
716
717---
718 src/test/test_config.c |   24 ++++++++++++++++--------
719 1 file changed, 16 insertions(+), 8 deletions(-)
720
721diff --git a/src/test/test_config.c b/src/test/test_config.c
722index 4562baf..a7ce6bf 100644
723--- a/src/test/test_config.c
724+++ b/src/test/test_config.c
725@@ -240,13 +240,20 @@ test_config_write_to_data_subdir(void* arg)
726   const char* subdir = "test_stats";
727   const char* fname = "test_file";
728   const char* str =
729-      "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod\n"
730-      "tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua.\n"
731-      "At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren,\n"
732-      "no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet,\n"
733-      "consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore\n"
734-      "magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et\n"
735-      "ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.";
736+      "Lorem ipsum dolor sit amet, consetetur sadipscing\n"
737+      "elitr, sed diam nonumy eirmod\n"
738+      "tempor invidunt ut labore et dolore magna aliquyam\n"
739+      "erat, sed diam voluptua.\n"
740+      "At vero eos et accusam et justo duo dolores et ea\n"
741+      "rebum. Stet clita kasd gubergren,\n"
742+      "no sea takimata sanctus est Lorem ipsum dolor sit amet.\n"
743+      "Lorem ipsum dolor sit amet,\n"
744+      "consetetur sadipscing elitr, sed diam nonumy eirmod\n"
745+      "tempor invidunt ut labore et dolore\n"
746+      "magna aliquyam erat, sed diam voluptua. At vero eos et\n"
747+      "accusam et justo duo dolores et\n"
748+      "ea rebum. Stet clita kasd gubergren, no sea takimata\n"
749+      "sanctus est Lorem ipsum dolor sit amet.";
750   const char* subpath = get_datadir_fname(subdir);
751   const char* filepath = get_datadir_fname2(subdir, fname);
752 
753@@ -260,7 +267,8 @@ test_config_write_to_data_subdir(void* arg)
754   test_assert(write_to_data_subdir(subdir, fname, str, NULL));
755   check_or_create_data_subdir(subdir);
756 
757-  // Content of file after write attempt should be equal to the original string.
758+  // Content of file after write attempt should be
759+  // equal to the original string.
760   test_assert(!write_to_data_subdir(subdir, fname, str, NULL));
761   test_streq(read_file_to_str(filepath, 0, NULL), str);
762 
763--
7641.7.9.5
765