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