Re: [tor-bugs] #1922 [Core Tor/Tor]: torrc.d-style configuration directories

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#1922: torrc.d-style configuration directories
-------------------------------------------------+-------------------------
 Reporter:  aa138346                             |          Owner:
                                                 |  Jigsaw52
     Type:  enhancement                          |         Status:
                                                 |  needs_review
 Priority:  Low                                  |      Milestone:  Tor:
                                                 |  0.3.1.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tor-client, intro,                   |  Actual Points:
  tor-03-unspecified-201612                      |
Parent ID:                                       |         Points:  medium
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by nickm):

 Looks like good solid work!  A few comments:

 1.
 {{{
 +          if (!list) {
 +            list = included_list;
 +            list_last = included_list_last;
 +          } else if (included_list) {
 +            list_last->next = included_list;
 +            list_last = included_list_last;
 +          }
 }}}

 This conditional isn't necessary -- look what we were doing with the
 "next" pointer in the old code.

 2. Can the code in the `if (!strcmp(k, "%include")) {` block be extracted
 into a new function?  It makes the config_get_lines_aux function huge, and
 it's logically fairly separate.

 3. There should probably be a flag to enable %include on specific files:
 we use config_get_lines() all over the place, and we don't necessarily
 want to allow includes in all of them.

 4. Maybe there should be two wrappers for config_get_lines_aux: one
 presenting the old API, and one presenting one the new API that allows
 includes and reports whether they have happened?  That way we could avoid
 changing all 22 places that call config_get_lines().

 5. This could be much shorter and safer with tor_asprintf.
 {{{
 +      size_t fullname_len = (size_t) (strlen(path) + strlen(f) + 2);
 +      char *fullname = tor_malloc_zero(fullname_len);
 +      strlcat(fullname, path, fullname_len);
 +      strlcat(fullname, PATH_SEPARATOR, fullname_len);
 +      strlcat(fullname, f, fullname_len);
 +      tor_free(f);
 }}}

 6. This could use a changes file; see doc/HACKING/CodingStandards.md for
 more info on those.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/1922#comment:74>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs