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(a)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/
As all Guenther's review issues were addressed, I pushed the patches to
master.