Make -initWithReachabilityRef: retain the ref argument, fixing a Clang analyzer warning. #414
Conversation
Without this Clang shows an analyzer warning that after calling -initWithReachabilityRef: the ref should be released. Because -initWithReachabilityRef: doesn’t retain the argument, this would result in a dangling pointer. Warning: This change fixes the issue in YapDatabase internally, but it introduces a memory leak for consumers that are currently using -initWithReachabilityRef without releasing the ref.
|
I'm not 100% sure that this should be merged. It is definitely the correct way to do it (in my opinion at least), but it will introduce a memory leak for consumers of the framework who are currently using -initWithReachabilityRef: and ignore the analyzer warning. On the other side it fixes a crash when someone actually "fixes" the analyzer warning in the consumer code by releasing the ref after passing it to the initializer. Ultimately, I don't think -initWithReachabilityRef: is used a lot directly, anyway. And when the convenience methods of YapReachability are used nothing changes (other than two warnings less). |
|
I'm inclined to agree with the changes made. Besides, if I have to choose between a memory leak and a crash, I'd have to go with the memory leak. |
Without this Clang shows an analyzer warning that after calling -initWithReachabilityRef: the ref should be released. Because -initWithReachabilityRef: doesn’t retain the argument, this would result in a dangling pointer.
Warning: This change fixes the issue in YapDatabase internally, but it introduces a memory leak for consumers that are currently using -initWithReachabilityRef without releasing the ref.