From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Brivio <sbrivio@redhat.com>
To: passt-dev@passt.top
Subject: Re: [PATCH 07/28] Clean up parsing in conf_runas()
Date: Wed, 28 Sep 2022 22:57:55 +0200
Message-ID: <20220928225755.6bfb59f9@elisabeth>
In-Reply-To: <20220928043339.613538-8-david@gibson.dropbear.id.au>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============1279008713484003927=="

--===============1279008713484003927==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

On Wed, 28 Sep 2022 14:33:18 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> conf_runas() handles several of the different possible cases for the
> --runas argument in a slightly odd order.  Although it can parse both
> numeric UIDs/GIDs and user/group names, it can't parse a numeric UID
> combined with a group name or vice versa.  That's not obviously useful, but
> it's slightly surprising gap to have.

Ah, actually, I had noticed that: I usually type my UID as number, but
I can't remember GIDs... then I had half a mind of "fixing" that, and
never got to it.

> [...]
>
> +++ b/conf.c
> @@ -859,46 +859,50 @@ dns6:
>   *
>   * Return: 0 on success, negative error code on failure
>   */
> -static int conf_runas(const char *opt, unsigned int *uid, unsigned int *gi=
d)
> +static int conf_runas(char *opt, unsigned int *uid, unsigned int *gid)

While I like your approach better than the existing one, I see one
detail that, albeit minor, might drive somebody mad: if the UID is
valid, but the GID isn't, we go back to conf_ugid():

		if (ret)
			err("Invalid --runas option: %s", runas);

and print the UID part, only, because we cut before the separator.

Perhaps we could simply put the separator back before returning
-EINVAL (instead of copying the buffer) -- we just have zero or one
occurrences of ':'.

--=20
Stefano


--===============1279008713484003927==--