[SSSD] [PATCH] sss_override: support import and export

Jakub Hrozek jhrozek at redhat.com
Mon Aug 17 19:47:45 UTC 2015


On Mon, Aug 17, 2015 at 10:32:00AM +0200, Jakub Hrozek wrote:
> On Sun, Aug 16, 2015 at 05:59:22PM +0200, Pavel Březina wrote:
> > https://fedorahosted.org/sssd/ticket/2737
> > 
> > Hi, it should work... :-) However, I wanted to make import as transaction so
> > no changes are made if some error occurs, but I had some troubles with it so
> > I gave up eventually.
> 
> Thanks a lot for the patches. I didn't do any review yet, just scrolled
> through them.
> 
> My first reaction was that the second patch should be split up and at
> least the colondb should be unit tested.
> 
> btw did you get inspired by some existing code, like libuser or
> shadow-utils with the colondb?

The patches are quite big, so maybe the review will be delivered in
chunks.

1) There are some Coverity errors:
Error: COMPILER_WARNING:
sssd-1.13.1/src/tools/common/sss_colondb.c:279:12: warning: 'fmode' may be used uninitialized in this function [-Wmaybe-uninitialized]
#         fp = fopen(filename, fmode);
#            ^
#  277|       if (filename != NULL) {
#  278|           errno = 0;
#  279|->         fp = fopen(filename, fmode);
#  280|           if (fp == NULL) {
#  281|               DEBUG(SSSDBG_CRIT_FAILURE, "Unable to open file %s\n", filename);

Error: RESOURCE_LEAK (CWE-772):
sssd-1.13.1/src/tools/common/sss_colondb.c:279: alloc_fn: Storage is returned from allocation function "fopen".
sssd-1.13.1/src/tools/common/sss_colondb.c:279: var_assign: Assigning: "fp" = storage returned from "fopen(filename, fmode)".
sssd-1.13.1/src/tools/common/sss_colondb.c:289: leaked_storage: Variable "fp" going out of scope leaks the storage it points to.
#  287|       if (db == NULL) {
#  288|           DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero() failed\n");
#  289|->         return NULL;
#  290|       }
#  291|   

Error: COMPILER_WARNING:
sssd-1.13.1/src/tools/common/sss_colondb.c: scope_hint: In function 'sss_colondb_open'
sssd-1.13.1/src/tools/common/sss_colondb.c:292:14: warning: 'fp' may be used uninitialized in this function [-Wmaybe-uninitialized]
#     db->file = fp;
#              ^
#  290|       }
#  291|   
#  292|->     db->file = fp;
#  293|       db->mode = mode;
#  294|   

Error: UNINIT (CWE-457):
sssd-1.13.1/src/tools/sss_override.c:997: var_decl: Declaring variable "tmp_ctx" without initializer.
sssd-1.13.1/src/tools/sss_override.c:1076: uninit_use_in_call: Using uninitialized value "tmp_ctx" when calling "_talloc_free".
# 1074|   
# 1075|   done:
# 1076|->     talloc_free(tmp_ctx);
# 1077|       return exit;
# 1078|   }

Error: UNINIT (CWE-457):
sssd-1.13.1/src/tools/sss_override.c:1207: var_decl: Declaring variable "tmp_ctx" without initializer.
sssd-1.13.1/src/tools/sss_override.c:1282: uninit_use_in_call: Using uninitialized value "tmp_ctx" when calling "_talloc_free".
# 1280|   
# 1281|   done:
# 1282|->     talloc_free(tmp_ctx);
# 1283|       return exit;
# 1284|   }

2) There is no documentation. It would be nice to have some examples in
the man page also.

3) I should probably test this before asking, but do the import and
export commands support blank (skipped) fields?



More information about the sssd-devel mailing list