>> +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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list