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