#30864: Move variable manipulation code out of confparse.c --------------------------+------------------------------------ Reporter: nickm | Owner: nickm Type: enhancement | Status: needs_review Priority: Medium | Milestone: Tor: 0.4.2.x-final Component: Core Tor/Tor | Version: Severity: Normal | Resolution: Keywords: | Actual Points: 2.5 Parent ID: #29211 | Points: 1 Reviewer: catalyst | Sponsor: Sponsor31-can --------------------------+------------------------------------
Comment (by catalyst): Thanks! Overall this is looking fairly good. I'm still having trouble with the vagueness of some of the descriptions in the explanatory comments. I started to write some line-by-line review comments on files in the pull request but found that I was having difficulty suggesting alternative text. So instead, I figured I should try to summarize (at a high level) what I'm finding confusing, in the hopes that it will make it easier to suggest alternative text in the pull request comments: * It's not clear what the main header file for this interface is. Maybe it's `typedvar.h`? * In many places, words like "type" and "variable" are used in vague ways, and not necessarily to refer to C types or variables. I kind of wish there were a high-level overview of how this works. My attempt to construct one from my analysis of the code is: ==== `typedvar.h` ==== * `typedvar.h` is an interface for accessing objects whose type is designated at run-time. (Maybe a kind of dynamic typing? Can we call it dynamic typing?) Access includes encoding, decoding, copying, clearing, etc. * `typedvar.h` functions use `void *` as generic pointers to the dynamically typed objects. (Maybe some opaque pointer type would be better, but then explicit casts would be needed.) * `var_type_def_t` objects are descriptors that instruct the `typedvar.h` functions how to manipulate a given dynamic object. * Most of the `typedvar.h` functions take pointers to `var_type_def_t` objects instead of enumerated type numbers to discover how to manipulate a given dynamically typed object. * Some `typedvar.h` functions take enumerated values from `config_type_t` to look up which `var_type_def_t` object to use. This seems like a bit of a layering violation but it's probably an acceptable intermediate situation. ==== `conftypes.h` ==== * `conftypes.h` holds the enumerated `config_type_t` values previously in `confparse.h`. `typedvar.h` functions use these enumerated values to look up which `var_type_def_t` describes a given dynamically typed object. ==== `type_defs.c` ==== * `type_defs.c` implements most of the functions that `typedvar.h` will use for accessing the dynamically typed objects. * `type_defs.c` also has the lookup table (and lookup function) to look up the appropriate `var_type_def_t` descriptor from a type number from `config_type_t`. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/30864#comment:11> 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