>> @@ -86,9 +86,12 @@ assemble_map (struct multipath * mp, char * params, int 
>> len)
>>          get_linux_version_code() < KERNEL_VERSION(4, 3, 0))
>>              add_feature(&mp->features, retain_hwhandler);
>>
>> -    f = STRDUP(mp->features);
> 
> clearly strdup()ing without checking if mp->features NULL is incorrect.
> However, I'm not sure that we need to fail if mp->features is NULL.
> instead, int the APPEND call, we could use the gcc ternary operator
> extension
> 
> (mp->features)?: "0"
> 
> to use "0" if mp->features is NULL.
> 
> Also, have you seen this actually occur?  Or is this just a theoretical
> issue that you've found from reading the code.  It seems like
> setup_map() will always call select_features() before calling
> assemble_map(), which should mean that mp->features will always be set
> in this case. Perhaps I'm missing something here.
> 
> -Ben
> 
Hi Ben,
  This just a theoretical issue and I did not see it. But it's not necessary
to call strdup. In your opinion, need multipath be checked?  I will make new
patch with your suggestion.

-Lixiaokeng
>> +    if (!mp->features) {
>> +            condlog(0, "mp->features is still NULL.");
>> +            goto err;
>> +    }



--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to