[Libvirt-cim] [PATCH] [CU] Commented some structures in the std_association.h file

[Kaitlin Rupert]

Richard Maciel wrote:
> # HG changeset patch
> # User Richard Maciel <richardm at br.ibm.com>
> # Date 1226604430 7200
> # Node ID b26dabfc31b3c528fe632c011ce76fd5426e0fd3
> # Parent  5fbf96fedcf7df32fccc3f989aa4520af8c9a264
> [CU] Commented some structures in the std_association.h file

You'll want to include the revision number of the patch in the subject 
each time you send an updated patch.  It's also good to include a 
comment block that states what changed between the patch versions.


> diff -r 5fbf96fedcf7 -r b26dabfc31b3 std_association.h

>  struct std_assoc {
>          char **source_class;
> +        /* Defines the list of possible classes that can be passed to the 
> +           association for this case */
> +
>          char *source_prop;
> +        /* Defines the property of the association class that refers 
> +           to the input (source class) of this case. This must match
> +           that of the schema, and is used for automatic generation of 
> +           the reference object in the References() or ReferenceNames()
> +           operation */
> 
>          char **target_class;
> +        /* Same as source_class, applied for target */

I would be a little more clear here - if the source class is "list of 
possible classes that can be passed to the association", then 
target_class is

  "list of possible classes that can be returned by the association for 
a given source_class list"

Or something like that.

> +
>          char *target_prop;
> +        /* Same as source_prop, applied for target */
>      
>          char **assoc_class;
> +        /* Defines the list of association classes which implement this
> +           association */

The association classes don't implement the association, the provider 
does that. So I would say something like:

"Defines the list of association classes which are implemented in this 
provider" or "implemented by this handler"

> 
>          assoc_handler_t handler;
> +        /* Function handler responsible for doing the association and
> +           returning the list of target instances of the association.
> +           The handler function receives the reference of the source 
> +           class of the association and must map it to a list of 
> +           CMPIInstance objects (targets of the association). */
> +
>          make_ref_t make_ref;
> +        /* Function handler responsible for creating an instance of the
> +           association class as requested by the References() or
> +           ReferenceNames() operation.  

For the handler field above, you don't mention the Associators() or 
AssociatorNames() calls.  I'd remove the mention of 
References()/ReferenceNames() because it clear from the rest of the comment.

> +           The handler function receives the source object path,
> +           and the target instance, so it can create the reference which is returned
> +           by the function. */
>  };
> 
> +/*
> + * The std_assoc_info is used to keep information related to the query done
> + * All members of this structure are named after the formal CIM association
> + * query components.
> + */

Instead of "of this structure are named after", I would say "of this 
structure contain the corresponding formal".

Not only are the elements named after them, but they are used to hold 
the query components themselves.

>  struct std_assoc_info {
>          const char *assoc_class;
>          const char *result_class;

-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com