Heidi Eckhart wrote:
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1196157587 -3600 # Node ID 690413b4aef454d03ce31003feea55a65fa5347b # Parent c3f590ec1553352439cbc6a884817ac13dc8eb40 Adoption of changes to RegisteredProfile Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com>
if (instance == NULL) { - CMSetStatusWithChars(_BROKER, &s, CMPI_RC_ERR_FAILED, + CMSetStatusWithChars(_BROKER, &s, + CMPI_RC_ERR_FAILED, "Can't create profile instance.");
Not sure why this hadn't occurred to me earlier, and I don't know if this has been brought up already, but now that we have cu_statusf, should we use that instead of CMSetStatusWithChars even in places where we don't need any fancy formatting? It helps to maintain consistency, and (although this is a bit of a minor point), it is a substantially shorter function name. I believe the calls are the exact same when you don't have any formatting to do.
goto error; } @@ -165,6 +172,8 @@ static CMPIStatus elem_to_prof(const CMP error: free(classname); out: + virConnectClose(conn); + return s; }
Paying more attention to how we free things now that we know virConnectClose is fine when given a NULL, wouldn't the same hold for classname? As long as it is initialized to NULL when it is declared, it can always be freed at the end like conn. I realize that classname was already like that before your patch (I think I wrote it that way actually), but while your fixing this function up it seems like a nice housekeeping thing. Optionally, we can go with "not relevant to the patch" and I'll try and clean up this type of thing wherever I can find it as a separate patch. -- -Jay