On 09/26/2012 02:40 PM, Dmitri Pal wrote:
Hi,

And again the same set + one patch that fixes Makefile.am




_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Gunther provided a review and found two issues.
--------------------------------------------------
In ini/ini_get_valueobj.c:ini_get_config_valueobj() line 132 the 
function returns "error" which is still set to EOK while I think EINVAL 
is more appropriate here:
 ini/ini_get_valueobj.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ini/ini_get_valueobj.c b/ini/ini_get_valueobj.c
index 6ed822e..6587e1d 100644
--- a/ini/ini_get_valueobj.c
+++ b/ini/ini_get_valueobj.c
@@ -129,7 +129,7 @@ int ini_get_config_valueobj(const char *section,
     /* Do we have a name ? */
     if (name == NULL) {
         TRACE_ERROR_NUMBER("Name is NULL it will not be found.", EINVAL);
-        return error;
+        return EINVAL;
     }
     /* Empty section means look for the default one */

    
Then I think there are some pointer dereferences, which theoretically 
can be NULL pointers and there are no explicit checks, like here in
ini/ini_list_valueobj.c:
 ini/ini_list_valueobj.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ini/ini_list_valueobj.c b/ini/ini_list_valueobj.c
index ef53e10..dc0992f 100644
--- a/ini/ini_list_valueobj.c
+++ b/ini/ini_list_valueobj.c
@@ -116,7 +116,7 @@ char **ini_get_attribute_list(struct ini_cfgobj 
*ini_config,
     /* Pass it to the function from collection API */
     list = col_collection_to_list(subcollection, size, error);
-
+    /* cant list be NULL at this point ? */
     col_destroy_collection(subcollection);
     /* Our list of attributes has a special extra attribute - remove it */
--------------------------------------------------------------------

Both are in patch 6.
At first I was not sure about the first issue but after a discussion with Stephen I decided to make a change.
The new patch set together with the coverity fix that was sent separately is included.

    
-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager for IdM portfolio
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/