[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #30864 [Core Tor/Tor]: Move variable manipulation code out of confparse.c

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