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@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@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