Re: [tor-bugs] #29209 [Core Tor/Tor]: Reduce visibility of more data type internals

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

#29209: Reduce visibility of more data type internals
----------------------------------------+----------------------------------
 Reporter:  nickm                       |          Owner:  (none)
     Type:  task                        |         Status:  needs_review
 Priority:  Medium                      |      Milestone:  Tor:
                                        |  0.4.1.x-final
Component:  Core Tor/Tor                |        Version:
 Severity:  Normal                      |     Resolution:
 Keywords:  technical-debt refactoring  |  Actual Points:  3.5
Parent ID:                              |         Points:  15
 Reviewer:  nickm                       |        Sponsor:  Sponsor31-can
----------------------------------------+----------------------------------

Comment (by nickm):

 Okay, I've read through the code.  This looks like a good start; I've got
 some suggestions before we merge.  First the minor ones:
   1. Let's open a sub-ticket for crypt_path_t, so that this ticket can
 still be about
   2. I think you're right that a bottom-up approach is better, where we
 start with low-level types like extend_info_t whose info is complex and
 exposed.  Your patch here does seem to demonstrate that to me.

 Now the major issue I have:

 I think that this "private struct" approach is not actually the best for
 the case where we want to make fields private one at a time.  It
 introduces an extra objects and an extra pointer, increasing both
 fragmentation and memory pressure.  Here are some other approaches that we
 could use that I think would create fewer code changes:
 {{{
 // Example

 struct foobar_t {
   int a;
   char *b; // let's say that we want to make this one private.
   long c;
 };


 // Option 1:

 struct foobar_t {
   int a;
   long c;
 };

 struct foobar_private_t {
   struct foobar_t base;
   char *b;
 };

 static inline foobar_private_t *
 to_private(foobar_t *obj)
 {
   char *ptr = (char *)obj;
   return (foobar_private_t *)(ptr - (OFFSET_OF(foobar_private_t, base)));
 }

 static inline foobar_t *
 to_public(foobar_private_t *obj)
 {
   return &obj->base;
 }

 // Option 2

 // (the foobar_private macro should only be defined in C modules that are
 // allowed to see the internals.)

 #ifdef FOOBAR_T_PRIVATE
 #define PRIV(x) x
 #else
 #define PRIV(x) foobar_private_member__ ## x ## __
 #endif

 struct foobar_t {
   int a;
   char *PRIV(b);
   long c;
 };

 // option 3

 #define PRIV(x) foobar_private_member__ ## x ## __

 struct foobar_t {
   int a;
   char *PRIV(b);
   long c;
 };

 #ifdef FOOBAR_T_PRIVATE
 #define b PRIV(b)
 #endif

 // option 4

 struct foobar_t {
   int a;
   long c;
   struct {
     char *b;
   } foobar_private_members__;
 };

 #ifdef FOOBAR_T_PRIVATE
 #define pvt foobar_private_members__
 #else
 #define foobar_private_members__ "you got a syntax error because you used
 this field outside foobar."
 #endif
 }}}

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/29209#comment:5>
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