Bug 702150 - Fix Coverity resource leaks
Summary: Fix Coverity resource leaks
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: 389
Classification: Retired
Component: Admin
Version: 1.2.8
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Nathan Kinder
QA Contact: Chandrasekar Kannan
URL:
Whiteboard:
Depends On:
Blocks: 434915
TreeView+ depends on / blocked
 
Reported: 2011-05-04 21:56 UTC by Nathan Kinder
Modified: 2015-01-04 23:48 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-05-17 14:08:34 UTC
Embargoed:


Attachments (Terms of Use)
Patch for cov#10829,10828,10827,10826 (20.74 KB, patch)
2011-05-04 22:02 UTC, Nathan Kinder
no flags Details | Diff
Patch for cov#10829,10828,10827,10826,10825,10824 (26.86 KB, patch)
2011-05-05 17:34 UTC, Nathan Kinder
nhosoi: review+
Details | Diff
Pat for cov#10823 (1.24 KB, patch)
2011-05-05 17:44 UTC, Nathan Kinder
nhosoi: review+
Details | Diff
Pat for cov#10822,10821 (1.95 KB, patch)
2011-05-05 17:56 UTC, Nathan Kinder
nhosoi: review+
Details | Diff
Pat for cov#10820,10819 (1.52 KB, patch)
2011-05-05 18:02 UTC, Nathan Kinder
nhosoi: review+
Details | Diff
Pat for cov#10818,10815,10814 (4.31 KB, patch)
2011-05-05 18:34 UTC, Nathan Kinder
nhosoi: review+
Details | Diff
Pat for cov#10816 (739 bytes, patch)
2011-05-05 18:39 UTC, Nathan Kinder
nhosoi: review+
Details | Diff
Pat for cov#10817 (894 bytes, patch)
2011-05-05 18:45 UTC, Nathan Kinder
nhosoi: review+
Details | Diff

Description Nathan Kinder 2011-05-04 21:56:09 UTC
A recent run of Coverity against adminserver turned up some resource leak issues that we need to address.

Comment 1 Nathan Kinder 2011-05-04 22:02:08 UTC
Created attachment 496932 [details]
Patch for cov#10829,10828,10827,10826

Comment 2 Nathan Kinder 2011-05-05 17:34:18 UTC
Created attachment 497173 [details]
Patch for cov#10829,10828,10827,10826,10825,10824

Comment 3 Nathan Kinder 2011-05-05 17:44:32 UTC
Created attachment 497176 [details]
Pat for cov#10823

Comment 4 Nathan Kinder 2011-05-05 17:56:37 UTC
Created attachment 497179 [details]
Pat for cov#10822,10821

Comment 5 Nathan Kinder 2011-05-05 18:02:13 UTC
Created attachment 497180 [details]
Pat for cov#10820,10819

Comment 6 Nathan Kinder 2011-05-05 18:34:51 UTC
Created attachment 497184 [details]
Pat for cov#10818,10815,10814

Comment 7 Nathan Kinder 2011-05-05 18:39:58 UTC
Created attachment 497186 [details]
Pat for cov#10816

Comment 8 Nathan Kinder 2011-05-05 18:45:28 UTC
Created attachment 497188 [details]
Pat for cov#10817

Comment 9 Noriko Hosoi 2011-05-05 19:17:57 UTC
Comment on attachment 497184 [details]
Pat for cov#10818,10815,10814

A bit curious why ds_get_conf_from_file does not call ds_free_config, but the code is doing the right thing.

Comment 10 Noriko Hosoi 2011-05-05 19:37:55 UTC
Comment on attachment 497173 [details]
Patch for cov#10829,10828,10827,10826,10825,10824

Could you double check 2 comments (one starting from line 1188 and another from line 1197) are not duplicated?  It might be a diff tool issue, I guess...

https://bugzilla.redhat.com/attachment.cgi?id=497173&action=diff#a/admserv/cgi-src40/htmladmin.c_sec8

Comment 11 Nathan Kinder 2011-05-05 20:44:40 UTC
(In reply to comment #9)
> Comment on attachment 497184 [details]
> Pat for cov#10818,10815,10814
> 
> A bit curious why ds_get_conf_from_file does not call ds_free_config, but the
> code is doing the right thing.

I really added ds_free_config() as a function that goes along with ds_get_config().  The ds_get_conf_from_file() function is the function that is used beneath ds_get_config() to actually build the list.  Since this function has the logic of the internals of the way the list is allocated, I made it just do the deallocation itself too.

Comment 12 Nathan Kinder 2011-05-05 20:52:37 UTC
(In reply to comment #10)
> Comment on attachment 497173 [details]
> Patch for cov#10829,10828,10827,10826,10825,10824
> 
> Could you double check 2 comments (one starting from line 1188 and another from
> line 1197) are not duplicated?  It might be a diff tool issue, I guess...
> 
> https://bugzilla.redhat.com/attachment.cgi?id=497173&action=diff#a/admserv/cgi-src40/htmladmin.c_sec8

I moved the comment to the top of the code block, but forgot to remove the original comment.  I'll remove the duplicate before I push the patch.

Comment 13 Nathan Kinder 2011-05-05 20:57:14 UTC
Pushed patches to master.  Thanks to Noriko for her reviews!

Counting objects: 62, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (43/43), done.
Writing objects: 100% (43/43), 6.45 KiB, done.
Total 43 (delta 32), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/admin.git
   ab4472e..b427aa1  master -> master


Note You need to log in before you can comment on or make changes to this bug.