[tor-bugs] #24337 [Core Tor/Tor]: Every _free() function should be a macro that sets the corresponding pointer to NULL.

[Tor Bug Tracker & Wiki]

#24337: Every _free() function should be a macro that sets the corresponding
pointer to NULL.
-----------------------------+------------------------------------
 Reporter:  nickm            |          Owner:  nickm
     Type:  enhancement      |         Status:  needs_review
 Priority:  Medium           |      Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor     |        Version:
 Severity:  Normal           |     Resolution:
 Keywords:  review-group-26  |  Actual Points:
Parent ID:                   |         Points:
 Reviewer:                   |        Sponsor:  Sponsor8-can
-----------------------------+------------------------------------

Comment (by asn):

 Hey, are we really sure we want to do this `tor_free() -> TOR_FREE()`
 substitution? It looks kind of alien.

 I wonder if the benefits outweight the ugliness here. I can probably get
 used to it given a bit of time, but I'm wondering if this is really
 necessary at this point. I haven't heard of many (if any) people get
 confused by this `tor_free` is macro, `tor_free_()` is function situation,
 and perhaps we could better resolve this confusion with a function doc on
 top of `tor_free()`.

 Furthermore, this seems like a pretty substantial readability/code-style
 change that has nothing to do with the original ticket purpose. Also, we
 have more macros that are lowercase (e.g. `circuit_mark_for_close()`,
 `connection_mark_for_close()`, `get_primary_or_port()`, etc.), do we
 really plan to turn these all to capitals?

 {{{
 $ grep -ER "#define [a-z]+" src/or/* | wc -l
 84
 }}}

 Do other people find this awkward, or do people like this code-style
 change?
 What are the benefits and what are the drawbacks?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/24337#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online