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

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

sssd-devel mailing list
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 | 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 
     /* Pass it to the function from collection API */
     list = col_collection_to_list(subcollection, size, error);
+    /* cant list be NULL at this point ? */
     /* 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?