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.