svn commit: r208332 - in head/sys: amd64/include i386/include

[Bruce Evans]

On Thu, 20 May 2010, Poul-Henning Kamp wrote:

> Log:
>  Rename an argument from "exp" to "expect" since the former makes FlexeLint
>  uneasy, in case anybody think it might be exp(3) in libm.

Bug in FlexeLint.

exp(3) is supposed to be in libc, so conflicts with it are more possible
than with a function in a pure independent library, but even then exp is
only reserved at certain scopes that don't apply here, and this reservation
actually reduces the problem (since exp is reserved as a macro iff
<math.h> is included, applications cannot #define it then and shouldn't
#define it otherwise).

<machine/atomic.h> should be kernel-only.

>  This also makes it consistent with other archs.

Bug in the other arches.

> Modified: head/sys/amd64/include/atomic.h
> ==============================================================================
> --- head/sys/amd64/include/atomic.h	Thu May 20 06:16:13 2010	(r208331)
> +++ head/sys/amd64/include/atomic.h	Thu May 20 06:18:03 2010	(r208332)
> @@ -76,8 +76,8 @@
> void atomic_##NAME##_##TYPE(volatile u_##TYPE *p, u_##TYPE v);	\
> void atomic_##NAME##_barr_##TYPE(volatile u_##TYPE *p, u_##TYPE v)
>
> -int	atomic_cmpset_int(volatile u_int *dst, u_int exp, u_int src);
> -int	atomic_cmpset_long(volatile u_long *dst, u_long exp, u_long src);
> +int	atomic_cmpset_int(volatile u_int *dst, u_int expect, u_int src);
> +int	atomic_cmpset_long(volatile u_long *dst, u_long expect, u_long src);
> u_int	atomic_fetchadd_int(volatile u_int *p, u_int v);
> u_long	atomic_fetchadd_long(volatile u_long *p, u_long v);
>

Function parameters are in thir own namespace, so they cannot conflict
with the extern function `exp', and one named `exp' is less likely to
conflict with anything than one name `p' for example.

> @@ -124,13 +124,13 @@ struct __hack
> /*
>  * Atomic compare and set, used by the mutex functions
>  *
> - * if (*dst == exp) *dst = src (all 32 bit words)
> + * if (*dst == expect) *dst = src (all 32 bit words)
>  *
>  * Returns 0 on failure, non-zero on success
>  */
>
> static __inline int
> -atomic_cmpset_int(volatile u_int *dst, u_int exp, u_int src)
> +atomic_cmpset_int(volatile u_int *dst, u_int expect, u_int src)

This API not only should be kernel-only; it obviously is kernel-only
(despite abuses in libc and hopefully nowhere else), since like most
kernel-only headers it takes no care with namespaces -- here it uses
the popular identifiers dst and src.

Style bug: all the parameters had short names of the same length.
Now `expect' is odd.

The atomic API is also kernel-only in its man page, and its man page
uses quite different parameter names to the above, so no change to the
man page is needed to be compatible with this commit, but either the
code or the man page should be changed to be compatible with each
other.  In the man page, the parameter names for the above function
are (dst, old, new); the man page has no reference to either `exp' or
`expect'.  Both sets are especially incompletely descriptive for the
old/expect parameter because the semantics of this parameter are
especially complicated so they cannot be described in 1 word.

Bruce