Jay Gagnon wrote:
Heidi Eckhart wrote:
> + if (s.rc != CMPI_RC_OK)
> + return s;
>
>
Sorry for being so nitpicky, but for the sake of consistency I think
this is a place where we use a "goto out;" type of statement as opposed
to having multiple returns.
I wouldn't call it nitpicky. Its a very good finding :) ! Thanks.
> +
> + prop = cu_compare_ref(ref, inst);
> + if (prop != NULL) {
> + cu_statusf(_BROKER, &s,
> + CMPI_RC_ERR_NOT_FOUND,
> + "No such instance (%s)", prop);
> + }
> +
>
>
Putting out right here should work fine.
Agree and done so in the updated patch.
Actually, this is more than a
nitpick I think, because if the above return happens we don't hit this
free() and leak classname.
Mhh ... the above code does only set the status, but does not return
right afterwards. So we wouldn't leak classname. Or did I miss something ?
> + free(classname);
> +
> + return s;
> +}
>
>
--
Regards
Heidi Eckhart
Software Engineer
IBM Linux Technology Center - Open Hypervisor