#1448 sss_seed tool review issues
Closed: Fixed None Opened 7 years ago by pbrezina.

These are issues that I found during review of Nick's sss_seed tool. We weren't able to get them to beta 6 in time so I create a separate ticket, because it's mainly about code style and talloc context and it is not critical.

sss_seed.c:52 - remove additional line

seed_init():
When you are assigning talloc context to a member of a structure, you should use the structure as a parent. Example:

Correct:
sctx->uctx = talloc_zero(sctx, struct user_ctx);
talloc_free(sctx) will free uctx as well.

Wrong:
sctx->uctx->name = talloc_strdup(sctx, pc_name);
talloc_free(sctx->uctx) will not free name.

Correct:
sctx->uctx->name = talloc_strdup(*uctx*, pc_name);
talloc_free(sctx->uctx) will free name.

seed_interactive_input():
Well, this is just awful. Why not do it this way?

    seed_interactive_input(*mem_ctx, *uctx, **_new_uctx) {
        struct uctx *new_uctx = talloc_zero(NULL, struct uctx);

        if (uctx->name == NULL) {
            ret = seed_str_input(new_uctx, ..., &new_uctx->name);
        } else {
            new_uctx->name = talloc_strdup(new_uctx, uctx->name);
        }

        done:
            if (ret == EOK) {
                *_new_uctx = talloc_steal(mem_ctx, new_uctx)
            } else {
                talloc_free(new_uctx);
            }
    }

    ret = seed_interactive_input(sctx, uctx, &new_uctx);
    if (ret != EOK) { fail }
    talloc_zfree(uctx);

seed_domain_user_info():
Remove mem_ctx parameter (you don't use it).
You are once using 'name' and once 'sctx->uctx->name'. Pick one. The best way would be to take input parameters username, domainname, sysdb and output parameter is_cached.

sss_seed.c:793:
 - steal grouplist on uctx:
   sctx->uctx->addgroups = talloc_steal(sctx->uctx, groups_list);
 - you don't need to test for NULL, talloc_steal(ctx, ptr) always returns ptr

sss_seed.c:886 - missing \n in debug message

sss_seed.c:907 - invalid indentation

Fields changed

owner: somebody => nguay

Master: 6ea6ec5

resolution: => fixed
status: new => closed

Fields changed

milestone: NEEDS_TRIAGE => SSSD 1.9.0 beta 6
rhbz: => 0
type: defect => task

Metadata Update from @pbrezina:
- Issue assigned to nguay
- Issue set to the milestone: SSSD 1.9.0 beta 6

2 years ago

Login to comment on this ticket.

Metadata