[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