#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@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs