git: 8d97e469dee9 - stable/13 - libthr: thr_attr.c: EINVAL, not ENOTSUP, on invalid arguments

The branch stable/13 has been updated by olce:

URL: https://cgit.FreeBSD.org/src/commit/?id=8d97e469dee9db347a6cf433ac7e4d40f36d2c48

commit 8d97e469dee9db347a6cf433ac7e4d40f36d2c48
Author:     Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2023-11-24 21:21:16 +0000
Commit:     Olivier Certner <olce@FreeBSD.org>
CommitDate: 2024-01-18 21:09:43 +0000

    libthr: thr_attr.c: EINVAL, not ENOTSUP, on invalid arguments
    
    On first read, POSIX may seem ambiguous about the return code for some
    scheduling-related pthread functions on invalid arguments.  But a more
    thorough reading and a bit of standards archeology strongly suggests
    that this case should be handled by EINVAL and that ENOTSUP is reserved
    for implementations providing only part of the functionality required by
    the POSIX option POSIX_PRIORITY_SCHEDULING (e.g., if an implementation
    doesn't support SCHED_FIFO, it should return ENOTSUP on a call to, e.g.,
    sched_setscheduler() with 'policy' SCHED_FIFO).
    
    This reading is supported by the second sentence of the very definition
    of ENOTSUP, as worded in CAE/XSI Issue 5 and POSIX Issue 6: "The
    implementation does not support this feature of the Realtime Feature
    Group.", and the fact that an additional ENOTSUP case was added to
    pthread_setschedparam() in Issue 6, which introduces SCHED_SPORADIC,
    saying that pthread_setschedparam() may return it when attempting to
    dynamically switch to SCHED_SPORADIC on systems that doesn't support
    that.
    
    glibc, illumos and NetBSD also support that reading by always returning
    EINVAL, and OpenBSD as well, since it always returns EINVAL but the
    corresponding code has a comment suggesting returning ENOTSUP for
    SCHED_FIFO and SCHED_RR, which it effectively doesn't support.
    
    Additionally, always returning EINVAL fixes inconsistencies where EINVAL
    would be returned on some out-of-range values and ENOTSUP on others.
    
    Reviewed by:            markj
    Approved by:            markj (mentor)
    MFC after:              2 weeks
    Sponsored by:           The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D43006
    
    (cherry picked from commit 0eccb45979a8ee3129e11b638ebc4cfa00942b80)
    
    Approved by:            markj (mentor)
---
 lib/libthr/thread/thr_attr.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/lib/libthr/thread/thr_attr.c b/lib/libthr/thread/thr_attr.c
index decbcb949167..0ccc31b22c13 100644
--- a/lib/libthr/thread/thr_attr.c
+++ b/lib/libthr/thread/thr_attr.c
@@ -380,13 +380,11 @@ int
 _thr_attr_setinheritsched(pthread_attr_t *attr, int sched_inherit)
 {
 
-	if (attr == NULL || *attr == NULL)
+	if (attr == NULL || *attr == NULL ||
+	    (sched_inherit != PTHREAD_INHERIT_SCHED &&
+	    sched_inherit != PTHREAD_EXPLICIT_SCHED))
 		return (EINVAL);
 
-	if (sched_inherit != PTHREAD_INHERIT_SCHED &&
-	    sched_inherit != PTHREAD_EXPLICIT_SCHED)
-		return (ENOTSUP);
-
 	(*attr)->sched_inherit = sched_inherit;
 	return (0);
 }
@@ -400,18 +398,15 @@ _thr_attr_setschedparam(pthread_attr_t * __restrict attr,
 {
 	int policy;
 
-	if (attr == NULL || *attr == NULL)
+	if (attr == NULL || *attr == NULL || param == NULL)
 		return (EINVAL);
 
-	if (param == NULL)
-		return (ENOTSUP);
-
 	policy = (*attr)->sched_policy;
 
 	if (policy == SCHED_FIFO || policy == SCHED_RR) {
 		if (param->sched_priority < _thr_priorities[policy-1].pri_min ||
 		    param->sched_priority > _thr_priorities[policy-1].pri_max)
-			return (ENOTSUP);
+			return (EINVAL);
 	} else {
 		/*
 		 * Ignore it for SCHED_OTHER now, patches for glib ports
@@ -432,10 +427,9 @@ int
 _thr_attr_setschedpolicy(pthread_attr_t *attr, int policy)
 {
 
-	if (attr == NULL || *attr == NULL)
+	if (attr == NULL || *attr == NULL ||
+	    policy < SCHED_FIFO || policy > SCHED_RR)
 		return (EINVAL);
-	if (policy < SCHED_FIFO || policy > SCHED_RR)
-		return (ENOTSUP);
 
 	(*attr)->sched_policy = policy;
 	(*attr)->prio = _thr_priorities[policy-1].pri_default;