[stunnel-users] fix resource leaks and potential null

Tue Feb 5 23:34:04 CET 2013

I will have to concede my point based on Jochen's reply. Setting it equal
to NULL initially would be the guaranteed way that the NULL check would
work. However, in all likelihood this would probably never be a problem
here.

On Tuesday, February 5, 2013, Brian Wilkins wrote:

> Looking at the addition of the close(fd), that is fine but when main()
> exits, the file descriptors will effectively be closed. Stylistically, its
> best to close them when you are done. Up to Michal if he wants to fix that
> part.
> On Feb 5, 2013 2:45 PM, "Brian Wilkins" <bwilkins at gmail.com<javascript:_e({}, 'cvml', 'bwilkins at gmail.com');>>
> wrote:
>
>> Of course if we did dereference it, the program would likely segfault.
>>
>> I have not had a chance to look at the other issues, but I will soon.
>> On Feb 5, 2013 2:31 PM, "Arthur Mesh" <arthurmesh at gmail.com<javascript:_e({}, 'cvml', 'arthurmesh at gmail.com');>>
>> wrote:
>>
>>> On Tue, Feb 05, 2013 at 01:52:12PM -0500, Brian Wilkins wrote:
>>> > You are completely correct in that regard, but checking if a pointer is
>>> > null is a different concept. It's not as if the program tried to see
>>> if it
>>> > actually contained a string.
>>>
>>> I am not sure why you are bringing up the concept of empty string here.
>>> I don't think it's relevant at all. Yes, errstr points to a string, but
>>> we
>>> don't dereference it check first char is not \0.
>>>
>>> In any case, looking further in to this defect, I believe it's false
>>> positive. The following bit from coverity is bogus:
>>>
>>> At (20): Condition "new_service_options.next", taking true branch
>>> 2096    if(new_service_options.next) { /* daemon mode: initialize
>>> sections */
>>> At (21): Condition "section", taking false branch
>>> 2097        for(section=new_service_options.next; section;
>>> section=section->next) {
>>> 2098            s_log(LOG_INFO, "Initializing service [%s]",
>>> section->servname);
>>> 2099            errstr=parse_service_option(CMD_END, section, NULL,
>>> NULL);
>>> 2100            if(errstr)
>>> 2101                break;
>>> 2102        }
>>> At (22): Falling through to end of if statement
>>> 2103    }
>>>
>>> Note that "At (21): Condition "section", taking false branch" can never
>>> be true, since section is guaranteed to be non-NULL (due to
>>> section==new_service_options.next, whereas new_service_options.next !=
>>> NULL).
>>>
>>> Thoughts?
>>>
>>> What about the other two UNINIT defects?  I assume memory leak defects
>>> are pretty obvious.
>>>
>>> Thanks
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.stunnel.org/pipermail/stunnel-users/attachments/20130205/16657c3a/attachment.html>