[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH REPOST 1/2] admin: Introduce virAdmClientClose API



>> +
>> +/**
>> + * virAdmClientClose:
>> + * @client: a valid client object reference
>> + * @flags: extra flags; not used yet, so callers should always pass 0
>> + *
>> + * Close @client's connection to daemon forcefully.
>> + *
>> + * Returns 0 if the daemon's connection with @client was closed successfully
>> + * or -1 in case of an error.
>> + */
>> +int virAdmClientClose(virAdmClientPtr client,
>> +                      unsigned int flags)
>> +{
>> +    int ret = -1;
>> +
>> +    VIR_DEBUG("client=%p, flags=%x", client, flags);
>> +    virResetLastError();
>> +
>> +    virCheckAdmClientGoto(client, error);
>> +    virCheckFlagsGoto(0, error);
> 
> Ewww. No. This should be checked on the server side. Not client.
> 

Well, this approach was used with all the other APIs as well. In my
opinion, we don't necessarily need to contact the remote daemon to check
for the flags at this moment, since we explicitly document, that we
don't support any flags yet. Once we change that and start supporting
any flags, it will have to be removed of course, with all the remaining
occurrences in libvirt-admin.c and the doc string will need to be
modified anyway. Another thing is, if callers would like to use some
flags, they'll most probably use our enumerated types or constants for
that, for which they will have to update the admin library and once they
update, the check will be gone.

Erik


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]