[libvirt] [PATCH 5/7] virnetserverclient: Add an internal method to retrieve client's identity

[Erik Skultety]

>> +int
>> +virNetServerClientGetInfo(virNetServerClientPtr client,
>> +                          bool *readonly, const char **sock_addr,
>> +                          virIdentityPtr *identity)
>> +{
>> +    int ret = -1;
>> +
>> +    virObjectLock(client);
>> +    *readonly = client->readonly;
>> +
>> +    if (!(*sock_addr = virNetServerClientRemoteAddrString(client))) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       _("No network socket associated with client"));
>> +        goto cleanup;
>> +    }
>> +
>> +    *identity = NULL;
>> +    if (client->identity)
>> +        *identity = virObjectRef(client->identity);
> 
> Should we drop this if() and therefore set *identity to NULL if there's
> none? I guess it's better to be safe than sorry. Moreover, if we don't
> do that and return 0 it's hard for callers of this function to determine
> if this argument has been updated or not. In general I think that
> function should either update all args or none.
> 

Not sure I got your point. If no identity is present, pointer will be
set to NULL and 0 is returned, so I'm not sure about the safe and sorry
part. Anyway, I do agree that we shouldn't touch caller's args if an
error occurs and since not having any identity information for a client
clearly is one, I suggest removing "*identity = NULL" and checking if
client identity exists at all and if not, returning -1 and raising an
error that no identity was available for a client...would that work for you?

>> +
>> +    ret = 0;
>> + cleanup:
>> +    virObjectUnlock(client);
>> +    return ret;
>> +}
>> diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h
>> index b576fde..2fbf60c 100644
>> --- a/src/rpc/virnetserverclient.h
>> +++ b/src/rpc/virnetserverclient.h
>> @@ -148,5 +148,8 @@ int virNetServerClientSendMessage(virNetServerClientPtr client,
>>  
>>  bool virNetServerClientNeedAuth(virNetServerClientPtr client);
>>  int virNetServerClientGetTransport(virNetServerClientPtr client);
>> +int virNetServerClientGetInfo(virNetServerClientPtr client,
>> +                              bool *readonly, const char **sock_addr,
>> +                              virIdentityPtr *identity);
>>  
>>  #endif /* __VIR_NET_SERVER_CLIENT_H__ */
>>
> 
> Don't forget to export this symbol in src/libvirt_remote.syms too.
> 
> Michal
> 

Yep, will add the symbol, thanks.

Erik