[Libvirt-cim] [PATCH 2 of 3] Adoption of changes to RegisteredProfile
Jay Gagnon
grendel at linux.vnet.ibm.com
Tue Nov 27 16:24:04 UTC 2007
Heidi Eckhart wrote:
> # HG changeset patch
> # User Heidi Eckhart <heidieck at linux.vnet.ibm.com>
> # Date 1196157587 -3600
> # Node ID 690413b4aef454d03ce31003feea55a65fa5347b
> # Parent c3f590ec1553352439cbc6a884817ac13dc8eb40
> Adoption of changes to RegisteredProfile
> Signed-off-by: Heidi Eckhart <heidieck at 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
More information about the Libvirt-cim
mailing list