[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 dgoulet):

 I'm going to add my opinion to this.

 There are cases (and maybe not standardize) where lowercase makes sense.
 For me, it does when the macro acts as a "function" that kind of could be
 a `static inline` but can't because we need to handle local variables
 (`tor_free()` for instance). And the lower case macro don't need to have
 pre-processor tricks applied to it (like doing arithmetic in it).

 I think there is a separation of semantic between a lower and upper case
 macro and Tor kind of follows it as "acting as a function" vs constant or
 inline testing a value (ex: `ERRNO_IS_EAGAIN()`).

 The case of `tor_free()` for me is a clear Tor wrapper around libc
 functions to make them safer and we have a many of those but they are
 functions. And I would prefer that it stays lower case for the semantic
 here linking that to our other wrappers and as `tor_free_()` is a
 function. Furthermore, there is a "if(predict_likely)" in there so one
 more argument in my opinion to treat it as a "function" that does have
 conditions and is a wrapper over the libc free().

 Finally, this change will hurt the `git blame` for no gain imho. We'll
 actually bring confusion and more syntax split with the current code base
 as asn pointed out our other lower case macros (`approx_time()` is a big
 one.)

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