[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, review-group-27  |  Actual Points:
Parent ID:                                    |         Points:
 Reviewer:                                    |        Sponsor:
                                              |  Sponsor8-can
----------------------------------------------+----------------------------

Comment (by dgoulet):

 I do like this as a safety measure. And code looks reasonable.

 Questions and comments:

 First thing. Can't we pass the typename instead of "some part of the type
 name" and then append the `_t` for `FREE_AND_NULL()`? I got confused when
 I saw this in `hs_service.h` because "hs_service" is not a thing at all in
 the code:

 {{{
 #define hs_service_free(s) FREE_AND_NULL(hs_service, (s))
 }}}

 ... which also would probably allow to remove the `UNMATCHED` version?

 On a more general note, what I'm concerned about is the added complexity
 that any free function of any object will kind of require in the future to
 follow our "code standards". Probably not unreasonable to ask for this but
 we are talking about an interesting requirement for future code to match
 this template:

 {{{
 void a_free_(a)
 #define a_free(a) FREE_AND_NULL(<type of a>, (a))
 }}}

 Which basically creates two public symbols for the same job, one "safe"
 and one "unsafe" (for the definition of safe being source ptr to NULL).

 I'm personally fine adapting here especially if this whole exercise
 becomes our practice and help mitigate use-after-free in some ways but we
 should just take a moment and realize the future of our free functions :).

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