Daniel Veillard wrote on 10/22/2008 05:14 AM:
> thanks for the changes! I think the prototypes are good to go, maybe a couple
> tiny changes but I can make them myself based on the 'make rebuild'
> output in the doc directory
> functions in src/libvirt.c still need arguments checkings however, but
> I can add that later.
Are these changes worth a 4th round of submissions - or is this something that can be taken care of in a follow-up patch?
>> +/**
>> + * virEventHandleCallback: callback for receiving file handle events
>
> description just need to be moved after the parameters, can be
> trivially done later.
>
>> + * @fd: file handle on which the event occurred
>> + * @events: bitset of events from virEventHandleType constants
>> + * @opaque: user data registered with handle
>> + */
>> +typedef void (*virEventHandleCallback)(int fd, virEventHandleType events,
>> + void *opaque);
>> +/**
>> + * virConnectDomainEventRegister:
>> + * @conn: pointer to the connection
>> + * @cb: callback to the function handling domain events
>> + * @opaque: opaque data to pass on to the callback
>> + *
>> + * Adds a Domain Event Callback
>> + *
>> + * Returns 0 on success, -1 on failure
>> + */
>> +int
>> +virConnectDomainEventRegister(virConnectPtr conn,
>> + virConnectDomainEventCallback cb,
>> + void *opaque)
>> +{
>
> conn still need to be checked with the standard block
>
> if (!VIR_IS_CONNECT(conn)) {
> virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
>
> like every other public functions with a conn parameter in src/libvirt.c
> What if cb is NULL, that probably need to be rejected
>
>> + /* Registering for a domain callback will enable delivery by default */
>> + if (conn->driver && conn->driver->domainEventRegister)
>> + return conn->driver->domainEventRegister (conn, cb, opaque);
>> + return -1;
>> +}
>> +
>> +/**
>> + * virConnectDomainEventDeregister:
>> + * @conn: pointer to the connection
>> + * @cb: callback to the function handling domain events
>> + *
>> + * Removes a Domain Event Callback
>> + *
>> + * Returns 0 on success, -1 on failure
>> + */
>> +int
>> +virConnectDomainEventDeregister(virConnectPtr conn,
>> + virConnectDomainEventCallback cb)
>> +{
>
> same thing for VIR_IS_CONNECT and cb being NULL tests
>
> otherwise looks fine to me.
>
> Daniel
>
>
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list