On 11/28/12 15:31, Osier Yang wrote:
On 2012年11月28日 21:34, Ján Tomko wrote:
Also remove the pointles check for NULL in auth.cephx.secret.uuid, since this is a static array.
It's nice if there is log of coverity.
Error: FORWARD_NULL (CWE-476): libvirt-0.10.2/src/conf/storage_conf.c:447: cond_false: Condition "auth->username == NULL", taking false branch libvirt-0.10.2/src/conf/storage_conf.c:451: if_end: End of if statement libvirt-0.10.2/src/conf/storage_conf.c:455: cond_true: Condition "uuid == NULL", taking true branch libvirt-0.10.2/src/conf/storage_conf.c:455: var_compare_op: Comparing "uuid" to null implies that "uuid" might be null. libvirt-0.10.2/src/conf/storage_conf.c:455: cond_false: Condition "auth->secret.usage == NULL", taking false branch libvirt-0.10.2/src/conf/storage_conf.c:459: if_end: End of if statement libvirt-0.10.2/src/conf/storage_conf.c:461: var_deref_model: Passing null pointer "uuid" to function "virUUIDParse(char const *, unsigned char *)", which dereferences it. (The dereference is assumed on the basis of the 'nonnull' parameter attribute.) Error: NO_EFFECT (CWE-398): libvirt-0.10.2/src/conf/storage_conf.c:979: array_null: Comparing an array to null is not useful: "src->auth.cephx.secret.uuid != NULL".
...
This change forces both of "uuid" and "usage" to be specified.
But from the RNG schema:
<define name='sourceinfoauthsecret'> <element name='secret'> <choice> <attribute name='uuid'> <text/> </attribute> <attribute name='usage'> <text/> </attribute> </choice> </element> </define>
Means that it allows only one of them specified.
Hm, from the schema, it should error out if both of them are specified too. So either there is problem of either the schema or the codes.
I think we have to figure out if the schema is correct first.
Looking at the code in storage_backend_rbd.c it looks like if both are specified, only usage is taken into account: if (pool->def->source.auth.cephx.secret.uuid != NULL) { virUUIDFormat(pool->def->source.auth.cephx.secret.uuid, secretUuid); VIR_DEBUG("Looking up secret by UUID: %s", secretUuid); secret = virSecretLookupByUUIDString(conn, secretUuid); } if (pool->def->source.auth.cephx.secret.usage != NULL) { VIR_DEBUG("Looking up secret by usage: %s", pool->def->source.auth.cephx.secret.usage); secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_CEPH, pool->def->source.auth.cephx.secret.usage); } I'll send another version shortly. Jan