[SSSD] [PATCH] IFP: cached objects

Jakub Hrozek jhrozek at redhat.com
Thu Jun 18 14:58:59 UTC 2015


On Thu, Jun 18, 2015 at 02:12:58PM +0200, Pavel Březina wrote:
> On 06/02/2015 03:49 PM, Pavel Březina wrote:
> >On 06/02/2015 02:17 PM, Lukas Slebodnik wrote:
> >>On (02/06/15 11:18), Pavel Březina wrote:
> >>>On 05/26/2015 05:22 PM, Jakub Hrozek wrote:
> >>>>There is a compilation warning discovered by Coverity:
> >>It is gcc-5.1 warning and not coverity
> >>I can see them as well.
> >>
> >>>>Error: COMPILER_WARNING:
> >>>>sssd-1.12.90/src/responder/ifp/ifp_cache.c:118:13: warning:
> >>>>'base_dn' may be used uninitialized in this function
> >>>>[-Wmaybe-uninitialized]
> >>>>#  116|       }
> >>>>#  117|
> >>>>#  118|->     ldb_ret = ldb_search(sysdb_ctx_get_ldb(domain->sysdb),
> >>>>tmp_ctx, &result,
> >>>>#  119|                            base_dn, LDB_SCOPE_SUBTREE, attrs,
> >>>>#  120|                            "(&(objectClass=%s)(%s=TRUE))",
> >>>>class, SYSDB_CACHED);
> >>>>
> >>>>Error: COMPILER_WARNING:
> >>>>sssd-1.12.90/src/responder/ifp/ifp_cache.c:118:13: warning: 'class'
> >>>>may be used uninitialized in this function [-Wmaybe-uninitialized]
> >>>>#     ldb_ret = ldb_search(sysdb_ctx_get_ldb(domain->sysdb),
> >>>>tmp_ctx, &result,
> >>>>#             ^
> >>>>#  116|       }
> >>>>#  117|
> >>>>#  118|->     ldb_ret = ldb_search(sysdb_ctx_get_ldb(domain->sysdb),
> >>>>tmp_ctx, &result,
> >>>>#  119|                            base_dn, LDB_SCOPE_SUBTREE, attrs,
> >>>>#  120|                            "(&(objectClass=%s)(%s=TRUE))",
> >>>>class, SYSDB_CACHED);
> >>>>
> >>>>Error: COMPILER_WARNING:
> >>>>sssd-1.12.90/src/responder/ifp/ifp_cache.c: scope_hint: In function
> >>>>'ifp_cache_get_cached_objects'
> >>>>sssd-1.12.90/src/responder/ifp/ifp_cache.c:134:18: warning: 'path'
> >>>>may be used uninitialized in this function [-Wmaybe-uninitialized]
> >>>>#         paths[i] = ifp_cache_build_path(paths, type, domain,
> >>>>result->msgs[i]);
> >>>>#                  ^
> >>>>#  132|
> >>>>#  133|       for (i = 0; i < result->count; i++) {
> >>>>#  134|->         paths[i] = ifp_cache_build_path(paths, type,
> >>>>domain, result->msgs[i]);
> >>>>#  135|           if (paths[i] == NULL) {
> >>>>#  136|               ret = ENOMEM;
> >>>
> >>>I don't see how those warning happends... base_dn, class and paths is
> >>>initialized.
> >>It seems that gcc 5.1 optimize enum in some ways
> >>and expects that functions ifp_cache_build_path and
> >>ifp_cache_build_base_dn
> >>can return unitialized value.
> >>If you do not wan to add Defualt to switch then you can initialize
> >>local variabels in these functions.
> >>
> >>diff --git a/src/responder/ifp/ifp_cache.c
> >>b/src/responder/ifp/ifp_cache.c
> >>index 4e5f59c..949c3f9 100644
> >>--- a/src/responder/ifp/ifp_cache.c
> >>+++ b/src/responder/ifp/ifp_cache.c
> >>@@ -34,7 +34,7 @@ ifp_cache_build_base_dn(TALLOC_CTX *mem_ctx,
> >>                          enum ifp_cache_type type,
> >>                          struct sss_domain_info *domain)
> >>  {
> >>-    struct ldb_dn *base_dn;
> >>+    struct ldb_dn *base_dn = NULL;
> >>
> >>      switch (type) {
> >>      case IFP_CACHE_USER:
> >>@@ -54,7 +54,7 @@ ifp_cache_build_path(TALLOC_CTX *mem_ctx,
> >>                       struct sss_domain_info *domain,
> >>                       struct ldb_message *msg)
> >>  {
> >>-    char *path;
> >>+    char *path = NULL;
> >>
> >>      switch (type) {
> >>      case IFP_CACHE_USER:
> >>
> >>LS
> >Thanks, new patch is attached.
> 
> Jakub wanted me to do two little modifications:
> 1) use sysdb_error_to_errno for ldb errors
> 2) change attr name ifp_cached to ifpCached
> 
> 

ACK. CI also passed and Coverity reports no errors. I tested with
commands such as:

$ dbus-send --print-reply --system \
            --dest=org.freedesktop.sssd.infopipe \
            /org/freedesktop/sssd/infopipe/Users \
            org.freedesktop.sssd.infopipe.Users.FindByName \
            string:admin object

path "/org/freedesktop/sssd/infopipe/Users/ipaldap/397400000"

$ dbus-send --print-reply --system \
            --dest=org.freedesktop.sssd.infopipe \
            /org/freedesktop/sssd/infopipe/Users \
            org.freedesktop.sssd.infopipe.Cache.List

array [
]

$ dbus-send --print-reply --system \
            --dest=org.freedesktop.sssd.infopipe \
            /org/freedesktop/sssd/infopipe/Users/ipaldap/397400000 \
            org.freedesktop.sssd.infopipe.Cache.Object.Store

boolean true

$ dbus-send --print-reply --system \
           --dest=org.freedesktop.sssd.infopipe \
           /org/freedesktop/sssd/infopipe/Users \
           org.freedesktop.sssd.infopipe.Cache.List
           
array [ 
    object path "/org/freedesktop/sssd/infopipe/Users/ipaldap/397400000"
]
    
$ dbus-send --print-reply --system \
            --dest=org.freedesktop.sssd.infopipe \
            /org/freedesktop/sssd/infopipe/Users/ipaldap/397400000 \
            org.freedesktop.sssd.infopipe.Cache.Object.Remove
             
boolean true

$ dbus-send --print-reply --system \
            --dest=org.freedesktop.sssd.infopipe \
            /org/freedesktop/sssd/infopipe/Users \
            org.freedesktop.sssd.infopipe.Cache.List
array [
]

I also added them to the commit message.



More information about the sssd-devel mailing list