[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