On Thu, Aug 22, 2019 at 05:19:04PM +0200, Michal Privoznik wrote:
This function has funny approach to retvals. Document them more clearly. Signed-off-by: Michal Privoznik <mprivozn redhat com> --- src/security/security_selinux.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 9857223bbf..0523613d4a 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1257,9 +1257,20 @@ virSecuritySELinuxGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return 0; } -/* Attempt to change the label of PATH to TCON. If OPTIONAL is true, - * return 1 if labelling was not possible. Otherwise, require a label - * change, and return 0 for success, -1 for failure. */ +/** + * virSecuritySELinuxSetFileconImpl: + * @path: path to the file to set context on + * @tcon: target context to set + * @optional: whether to treat errors as fatal + * @privileged: whether running as privileged user + * + * Set @tcon SELinux context on @path. If unable to do so, check SELinux + * configuration and produce sensible error message suggesting solution. + * + * Returns: -1 if failed to set context and SELinux is in enforcing mode + * 1 if failed to set context and @optional is true + * 0 otherwise.
It's not really how it behaves right now. Not that it matters, but maybe fixing the function and then commenting it would be easier. I would prefer If you fixed up the description for return values related to review in PATCH 2 or squashed PATCHes 1 and 2 together. Either way: Reviewed-by: Martin Kletzander <mkletzan redhat com>
Description: PGP signature