[PATCH] Make convenience macros for some pthread_attr valuevalidation

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

Hi,

On review of my patch to add an API for default pthread attributes,
Roland suggested[1] that I make some of the validations for pthread
attributes into macros since they're repeated in the pthread_attr_set*
functions.  I figured this compaction can be done independently since
there already is some duplication of this code.  Attached patch makes
macros out of checks for validity of stacksize of a thread, scheduling
priority and scheduling policy.  Everything else either has not
duplication of validation or no validation at all.

The scheduling policy check is currently not duplicated, but it would
be with my API patch, which is why I put it into a macro in advance.

The scheduling priority validation is duplicated in pthread_create,
but the latter does not check for validity of return value of
sched_get_priority_{min,max}.  This is not correct since the input
scheduling policy parameter is not validated earlier.  Reusing the
VALID_SCHED_PRIO macro fixes this.

Tested on x86_64 to verify that there are no regressions resulting
from this.  OK to commit?

Siddhesh

[1] http://sourceware.org/ml/libc-alpha/2013-03/msg00365.html

	* pthreadP.h (VALID_SCHED_POLICY): New macro to valid a
	scheduling policy value.
	(VALID_SCHED_PRIO): New macro to validate scheduling priority
	for a policy.
	(VALID_STACKSIZE): New macro to valid thread stack size.
	* pthread_attr_setschedparam (__pthread_attr_setschedparam):
	Use VALID_SCHED_PRIO.
	* pthread_create.c (__pthread_create_2_1): Likewise.
	* pthread_attr_setschedpolicy.c
	(__pthread_attr_setschedpolicy): Use VALID_SCHED_POLICY.
	* pthread_attr_setstack.c (__pthread_attr_setstack): Use
	VALID_STACKSIZE.
	* pthread_attr_setstacksize.c (__pthread_attr_setstacksize):
	Likewise.

diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index d08b219..75242e7 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -589,4 +589,21 @@ extern void __wait_lookup_done (void) attribute_hidden;
 # define USE_REQUEUE_PI(mut) 0
 #endif
 
+/* Evaluates to true if POL is one of the accepted values for thread scheduling
+   policy.  */
+#define VALID_SCHED_POLICY(pol) \
+  ((pol) == SCHED_OTHER || (pol) == SCHED_FIFO || (pol) == SCHED_RR)
+
+/* Evaluates to true if PR is within the accepted range of priority values for
+   the scheduling policy POL.  */
+#define VALID_SCHED_PRIO(pr, pol) \
+  ({									      \
+    int min = sched_get_priority_min (pol);				      \
+    int max = sched_get_priority_max (pol);				      \
+    (min >= 0 && max >= 0 && (pr) >= min && (pr) <= max);		      \
+  })
+
+/* Evaluates to true if ST is a valid stack size.  */
+#define VALID_STACKSIZE(st) ((st) >= PTHREAD_STACK_MIN)
+
 #endif	/* pthreadP.h */
diff --git a/nptl/pthread_attr_setschedparam.c b/nptl/pthread_attr_setschedparam.c
index ec1a8bd..b11e912 100644
--- a/nptl/pthread_attr_setschedparam.c
+++ b/nptl/pthread_attr_setschedparam.c
@@ -30,10 +30,7 @@ __pthread_attr_setschedparam (attr, param)
   assert (sizeof (*attr) >= sizeof (struct pthread_attr));
   struct pthread_attr *iattr = (struct pthread_attr *) attr;
 
-  int min = sched_get_priority_min (iattr->schedpolicy);
-  int max = sched_get_priority_max (iattr->schedpolicy);
-  if (min == -1 || max == -1
-      || param->sched_priority > max || param->sched_priority < min)
+  if (!VALID_SCHED_PRIO (param->sched_priority, iattr->schedpolicy))
     return EINVAL;
 
   /* Copy the new values.  */
diff --git a/nptl/pthread_attr_setschedpolicy.c b/nptl/pthread_attr_setschedpolicy.c
index 2fa6921..b090e83 100644
--- a/nptl/pthread_attr_setschedpolicy.c
+++ b/nptl/pthread_attr_setschedpolicy.c
@@ -32,7 +32,7 @@ __pthread_attr_setschedpolicy (attr, policy)
   iattr = (struct pthread_attr *) attr;
 
   /* Catch invalid values.  */
-  if (policy != SCHED_OTHER && policy != SCHED_FIFO && policy != SCHED_RR)
+  if (!VALID_SCHED_POLICY (policy))
     return EINVAL;
 
   /* Store the new values.  */
diff --git a/nptl/pthread_attr_setstack.c b/nptl/pthread_attr_setstack.c
index 783cffe..40a84ac 100644
--- a/nptl/pthread_attr_setstack.c
+++ b/nptl/pthread_attr_setstack.c
@@ -39,7 +39,7 @@ __pthread_attr_setstack (attr, stackaddr, stacksize)
   iattr = (struct pthread_attr *) attr;
 
   /* Catch invalid sizes.  */
-  if (stacksize < PTHREAD_STACK_MIN)
+  if (!VALID_STACKSIZE (stacksize))
     return EINVAL;
 
 #ifdef EXTRA_PARAM_CHECKS
diff --git a/nptl/pthread_attr_setstacksize.c b/nptl/pthread_attr_setstacksize.c
index b7f2919..c6c47f5 100644
--- a/nptl/pthread_attr_setstacksize.c
+++ b/nptl/pthread_attr_setstacksize.c
@@ -37,7 +37,7 @@ __pthread_attr_setstacksize (attr, stacksize)
   iattr = (struct pthread_attr *) attr;
 
   /* Catch invalid sizes.  */
-  if (stacksize < PTHREAD_STACK_MIN)
+  if (!VALID_STACKSIZE (stacksize))
     return EINVAL;
 
   iattr->stacksize = stacksize;
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index c6f2fdd..84fb4c4 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -541,12 +541,8 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
 	}
 
       /* Check for valid priorities.  */
-      int minprio = INTERNAL_SYSCALL (sched_get_priority_min, scerr, 1,
-				      iattr->schedpolicy);
-      int maxprio = INTERNAL_SYSCALL (sched_get_priority_max, scerr, 1,
-				      iattr->schedpolicy);
-      if (pd->schedparam.sched_priority < minprio
-	  || pd->schedparam.sched_priority > maxprio)
+      if (!VALID_SCHED_PRIO (pd->schedparam.sched_priority,
+			     iattr->schedpolicy))
 	{
 	  /* Perhaps a thread wants to change the IDs and if waiting
 	     for this stillborn thread.  */