[libvirt] [PATCH 2/2] security: update user and group parsing in security_dac.c

[Peter Krempa]

On 10/06/12 04:52, Marcelo Cerri wrote:
> The functions virGetUserID and virGetGroupID are now able to parse
> user/group names and IDs in a similar way to coreutils' chown. So, user
> and group parsing in security_dac can be simplified.
> ---
>   src/security/security_dac.c | 40 ++++++++++------------------------------
>   1 file changed, 10 insertions(+), 30 deletions(-)
>
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index a427e9d..e976144 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -95,39 +95,19 @@ int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr)
>       group = sep + 1;
>
>       /* Parse owner */
> -    if (*owner == '+') {
> -        if (virStrToLong_ui(++owner, NULL, 10, &theuid) < 0) {
> -            virReportError(VIR_ERR_INVALID_ARG,
> -                           _("Invalid uid \"%s\" in DAC label \"%s\""),
> -                           owner, label);
> -            goto cleanup;
> -        }
> -    } else {
> -        if (virGetUserID(owner, &theuid) < 0 &&
> -            virStrToLong_ui(owner, NULL, 10, &theuid) < 0) {
> -            virReportError(VIR_ERR_INVALID_ARG,
> -                           _("Invalid owner \"%s\" in DAC label \"%s\""),
> -                           owner, label);
> -            goto cleanup;
> -        }
> +    if (virGetUserID(owner, &theuid) < 0) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("Invalid owner \"%s\" in DAC label \"%s\""),
> +                       owner, label);
> +        goto cleanup;
>       }
>
>       /* Parse group */
> -    if (*group == '+') {
> -        if (virStrToLong_ui(++group, NULL, 10, &thegid) < 0) {
> -            virReportError(VIR_ERR_INVALID_ARG,
> -                           _("Invalid gid \"%s\" in DAC label \"%s\""),
> -                           group, label);
> -            goto cleanup;
> -        }
> -    } else {
> -        if (virGetGroupID(group, &thegid) < 0 &&
> -            virStrToLong_ui(group, NULL, 10, &thegid) < 0) {
> -            virReportError(VIR_ERR_INVALID_ARG,
> -                           _("Invalid group \"%s\" in DAC label \"%s\""),
> -                           group, label);
> -            goto cleanup;
> -        }
> +    if (virGetGroupID(group, &thegid) < 0) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("Invalid group \"%s\" in DAC label \"%s\""),
> +                       group, label);

Hm, this will mask the errors from virGetGroupID that might be useful. 
Should we remove this message in favor of the underlying messages or 
have this that has more relevant information where to find the issue but 
maybe mask the reason?

Any opinions?

> +        goto cleanup;
>       }
>
>       if (uidPtr)
>


Peter