Re: [Libguestfs] [PATCH 2/3] daemon/selinux-relabel: search for "invalid option" in setfiles output

On Wed, Apr 26, 2023 at 04:37:21PM +0200, Laszlo Ersek wrote: > On 4/26/23 14:59, Andrey Drobyshev wrote: >> 'X' in the setiles' stderr doesn't necessarily mean that option 'X' >> doesn't exist. For instance, when passing '-T' we get: "setfiles: >> option requires an argument -- 'T'". >> >> Signed-off-by: Andrey Drobyshev <andrey.drobyshev(a)virtuozzo.com&gt; >> --- >> daemon/selinux-relabel.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/daemon/selinux-relabel.c b/daemon/selinux-relabel.c >> index 454486c17..60a6f48a9 100644 >> --- a/daemon/selinux-relabel.c >> +++ b/daemon/selinux-relabel.c >> @@ -56,8 +56,9 @@ setfiles_has_option (int *flag, char opt_char) >> >> if (*flag == -1) { >> char option[] = { '-', opt_char, '\0' }; /* "-X" */ >> - char err_opt[] = { '\'', opt_char, '\'', '\0'}; /* "'X'" */ >> + char err_opt[32]; /* "invalid option -- 'X'" */ >> >> + snprintf(err_opt, sizeof(err_opt), "invalid option -- '%c'", opt_char); >> ignore_value (command (NULL, &err, "setfiles", option, NULL)); >> *flag = err && strstr (err, /* "invalid option -- " */ err_opt) == NULL; >> } > > Can you check in the selinux library git history how far back the > > invalid option -- '%c' > > message can be relied upon? It actually comes from glibc: https://sourceware.org/git/?p=glibc.git;a=blob;f=posix/getopt.c;h=1e2441c... It's actually been around "forever" .. It exists in the initial import (to CVS?) from 1995: https://sourceware.org/git/?p=glibc.git;a=blob;f=posix/posix/getopt.c;h=7... and there's even a claim there about POSIX -- but it would only apply for POSIXLY_CORRECT which we don't use in the daemon.

But yes it looks safe! Rich. > > Other than that, I'd suggest a number of superficial updates, but for a > change, I won't obsess about them. > > series > Reviewed-by: Laszlo Ersek <lersek(a)redhat.com&gt; > > (We shouldn't merge this until Rich agrees, too.) > > Laszlo