#1448 sss_seed tool review issues
Closed: Fixed None Opened 8 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

4 years ago

SSSD is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in SSSD's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/SSSD/sssd/issues/2490

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Login to comment on this ticket.

Metadata