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 ? Sorry, my "above" was a bit vague. I meant that if the "return s;" at
Heidi Eckhart wrote: the very top of this email happens, we leave the function before we get to "free(classname);" and so classname will be leaked. But no matter, the point will become moot when you switch the return to a goto; classname definitely won't leak that way. -- -Jay