On 07/31/2013 02:16 PM, Lukas Slebodnik wrote:
On (31/07/13 10:57), Pavel Březina wrote:
On 07/30/2013 02:42 PM, Lukas Slebodnik wrote:
ehlo,
Ensure that cmd_ctx->name will not be NULL.
If cmd_ctx->name was not initialized by sss_parse_name
then copy of name will be used.
https://fedorahosted.org/sssd/ticket/1970
Coverity ID: 11647
Patch is attached.
LS
Nack.
if (cmd_ctx->name == NULL) {
cmd_ctx->name = talloc_strdup(cmd_ctx, name);
if (!cmd_ctx->name) return ENOMEM;
}
if (cmd_ctx->is_user && cmd_ctx->domname == NULL) {
DEBUG(SSSDBG_TRACE_FUNC,
("Parsing name [%s][%s]\n", name, domain ? domain : "<ALL>"));
You need to talloc_zfree(cmd_ctx->name) before reassigning it.
ret = sss_parse_name_for_domains(cmd_ctx, cctx->rctx->domains,
domain, name,
&cmd_ctx->domname,
&cmd_ctx->name);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
("Invalid name received [%s]\n", name));
return ENOENT;
}
} else if (cmd_ctx->domname == NULL) {
if (domain != NULL) {
cmd_ctx->domname = talloc_strdup(cmd_ctx, domain);
if (!cmd_ctx->domname) return ENOMEM;
}
}
if (alias != NULL && strcmp(cmd_ctx->name, alias) != 0) {
cmd_ctx->alias = talloc_strdup(cmd_ctx, alias);
if (!cmd_ctx->alias) return ENOMEM;
}
However, I think it would be read better this way:
if (cmd_ctx->is_user && cmd_ctx->domname == NULL) {
...
} else {
if (cmd_ctx->name == NULL) {
...
}
if (cmd_ctx->domname == NULL && domain != NULL) {
...
}
}
Thank you for review, It is a nice catch.
I should not write patch anymore in the meeting time.
Updated patch is attached.
LS
Ack.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
