On Wed, Jul 31, 2013 at 02:31:37PM +0200, Pavel Březina wrote:
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.
Pushed to master.