Re: [PATCH v3 44/48] util: change identity class attribute names

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

 



Daniel P. Berrangé writes:

> Change the identity class attribute names with a "UNIX" tag to have a
> more generic "OS" tag, since when we expose this in the public API we
> want it to be more flexible for the future.
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
>  src/access/viraccessdriverpolkit.c | 12 ++---
>  src/admin/admin_server.c           | 10 ++--
>  src/libvirt_private.syms           | 24 ++++-----
>  src/rpc/virnetserverclient.c       | 12 ++---
>  src/util/viridentity.c             | 84 +++++++++++++++---------------
>  src/util/viridentity.h             | 60 ++++++++++-----------
>  tests/viridentitytest.c            | 18 +++----
>  tests/virnetserverclienttest.c     |  8 +--
>  8 files changed, 114 insertions(+), 114 deletions(-)
>
> diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c
> index b1473cd0a4..b98122d4a3 100644
> --- a/src/access/viraccessdriverpolkit.c
> +++ b/src/access/viraccessdriverpolkit.c
> @@ -88,19 +88,19 @@ virAccessDriverPolkitGetCaller(const char *actionid,
>          return -1;
>      }
>
> -    if (virIdentityGetUNIXProcessID(identity, pid) < 0) {
> +    if (virIdentityGetOSProcessID(identity, pid) < 0) {
>          virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("No UNIX process ID available"));
> +                       _("No OS process ID available"));

What about "No process ID available"?

>          goto cleanup;
>      }
> -    if (virIdentityGetUNIXProcessTime(identity, startTime) < 0) {
> +    if (virIdentityGetOSProcessTime(identity, startTime) < 0) {
>          virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("No UNIX process start time available"));
> +                       _("No OS process start time available"));

What about "No process start time available"?

>          goto cleanup;
>      }
> -    if (virIdentityGetUNIXUserID(identity, uid) < 0) {
> +    if (virIdentityGetOSUserID(identity, uid) < 0) {
>          virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("No UNIX caller UID available"));
> +                       _("No OS caller UID available"));

What about "No caller user ID available"?
(the term "user ID" is used elsewhere in the code)

The UID acronym is not widely known outside of programmer circles,
and for those who know it, it's practically always related to Unix-like
systems (on Windows, it's generally called a SID).

>          goto cleanup;
>      }
>
> diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c
> index f2a38f6dfa..b92eb2fdc6 100644
> --- a/src/admin/admin_server.c
> +++ b/src/admin/admin_server.c
> @@ -257,29 +257,29 @@ adminClientGetInfo(virNetServerClientPtr client,
>          pid_t pid;
>          uid_t uid;
>          gid_t gid;
> -        if (virIdentityGetUNIXUserID(identity, &uid) < 0 ||
> +        if (virIdentityGetOSUserID(identity, &uid) < 0 ||

Even in function names, I believe that "OS" could be removed without
much of a loss in readability.


>              virTypedParamsAddInt(&tmpparams, nparams, &maxparams,
>                                   VIR_CLIENT_INFO_UNIX_USER_ID, uid) < 0)
>              goto cleanup;
>
> -        if (virIdentityGetUNIXUserName(identity, &attr) < 0 ||
> +        if (virIdentityGetOSUserName(identity, &attr) < 0 ||

The above comment applies even more for a user name.

>              virTypedParamsAddString(&tmpparams, nparams, &maxparams,
>                                      VIR_CLIENT_INFO_UNIX_USER_NAME,

There are some spots where "UNIX" remains...
>                                      attr) < 0)
>              goto cleanup;
>
> -        if (virIdentityGetUNIXGroupID(identity, &gid) < 0 ||
> +        if (virIdentityGetOSGroupID(identity, &gid) < 0 ||
>              virTypedParamsAddInt(&tmpparams, nparams, &maxparams,
>                                   VIR_CLIENT_INFO_UNIX_GROUP_ID, gid) < 0)
>              goto cleanup;
>
[...]
>
>      if (virIdentityGetAttr(ident,
> -                           VIR_IDENTITY_ATTR_UNIX_GROUP_ID,
> +                           VIR_IDENTITY_ATTR_OS_GROUP_ID,

... whereas it is changed at other places.

>                             &gotGroupID) < 0) {
>          fprintf(stderr, "Missing group ID in identity\n");
>          goto cleanup;
> --
> 2.21.0

No functional issue found.


Reviewed-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>

--
Cheers,
Christophe de Dinechin (IRC c3d)

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux