[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #1922 [Core Tor/Tor]: torrc.d-style configuration directories
#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