[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #24337 [Core Tor/Tor]: Every _free() function should be a macro that sets the corresponding pointer to NULL.
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs