svn commit: r261796 - head/lib/libkvm

[Bruce Evans]

On Wed, 12 Feb 2014, Gleb Smirnoff wrote:

> Log:
>  While it isn't too late and kvm_read_zpcpu() function isn't yet used
>  outside libkvm(3), change its order of arguments, so that it is the
>  same as in kvm_read().

This also fixes some but not all namespace pollution.

> Modified: head/lib/libkvm/kvm.h
> ==============================================================================
> --- head/lib/libkvm/kvm.h	Wed Feb 12 08:04:38 2014	(r261795)
> +++ head/lib/libkvm/kvm.h	Wed Feb 12 09:41:17 2014	(r261796)
> @@ -88,7 +88,7 @@ kvm_t	 *kvm_open
> kvm_t	 *kvm_openfiles
> 	    (const char *, const char *, const char *, int, char *);
> ssize_t	  kvm_read(kvm_t *, unsigned long, void *, size_t);
> -ssize_t	  kvm_read_zpcpu(kvm_t *, void *, u_long, size_t, int);

This shouldn't even have compiled, but someone broke kvm.h by changing its
include of <sys/_types.h> to <sys/types.h>.

> +ssize_t	  kvm_read_zpcpu(kvm_t *, unsigned long, void *, size_t, int);

This fixes one dependency on the namespace pollution.

> ssize_t	  kvm_write(kvm_t *, unsigned long, const void *, size_t);
> __END_DECLS
>
> Modified: head/lib/libkvm/kvm_getpcpu.3
> ==============================================================================
> --- head/lib/libkvm/kvm_getpcpu.3	Wed Feb 12 08:04:38 2014	(r261795)
> +++ head/lib/libkvm/kvm_getpcpu.3	Wed Feb 12 09:41:17 2014	(r261796)
> @@ -50,7 +50,7 @@
> .Ft void *
> .Fn kvm_getpcpu "kvm_t *kd" "int cpu"
> .Ft ssize_t
> -.Fn kvm_read_zpcpu "kvm_t *kd" "void *buf" "u_long base" "size_t size" "int cpu"
> +.Fn kvm_read_zpcpu "kvm_t *kd" "u_long base" "void *buf" "size_t size" "int cpu"

This doesn't fix the documentation saying to use the namespace pollution for
the changed function...

> .Ft uint64_t
> .Fn kvm_counter_u64_fetch "kvm_t *kd" "u_long base"

...or for other functions.

This bug was missing in both the header and the man page for all of the
older functions that use unsigned long (kvm_read(), kvm_uread() and
kvm_write()).

kvm.h otherwise depends on the full pollution of <sys/types.h> only for the
declaration uint64_t.  It should declare this itself like it does for all
the other non-underscored typedefed types that it uses.

> .Sh DESCRIPTION
>
> Modified: head/lib/libkvm/kvm_pcpu.c
> ==============================================================================
> --- head/lib/libkvm/kvm_pcpu.c	Wed Feb 12 08:04:38 2014	(r261795)
> +++ head/lib/libkvm/kvm_pcpu.c	Wed Feb 12 09:41:17 2014	(r261796)
> @@ -306,7 +306,7 @@ kvm_dpcpu_setcpu(kvm_t *kd, u_int cpu)
>  * Obtain a per-CPU copy for given cpu from UMA_ZONE_PCPU allocation.
>  */
> ssize_t
> -kvm_read_zpcpu(kvm_t *kd, void *buf, u_long base, size_t size, int cpu)
> +kvm_read_zpcpu(kvm_t *kd, u_long base, void *buf, size_t size, int cpu)
> {
>
> 	return (kvm_read(kd, (uintptr_t)(base + sizeof(struct pcpu) * cpu),
> @@ -327,7 +327,7 @@ kvm_counter_u64_fetch(kvm_t *kd, u_long
>
> 	r = 0;
> 	for (int i = 0; i < mp_ncpus; i++) {
> -		if (kvm_read_zpcpu(kd, &c, base, sizeof(c), i) != sizeof(c))
> +		if (kvm_read_zpcpu(kd, base, &c, sizeof(c), i) != sizeof(c))
> 			return (0);
> 		r += c;
> 	}
>

The implementation can reasonably use u_long after including
<sys/types.h> for itself.  In fact, it is a style bug to not do so.
In old versions, libkvm/*.c had 1 instance of the style bug 'unsigned
foo' and 105 instances of u_foo.  Now it is even cleaner -- it has 0
instances of the style bug and 196 instances of u_foo.  It mostly
includes <sys/types.h> by including <sys/param.h>.  One newer file
has the style bug of including both, and another newer file is
sophisticated and includes only <sys/types.h>.

kvm_getfiles(3) documents a prerequesite for <sys/types.h> bogusly.
This should be under the _KERNEL ifdef.  And the kernel ifdef shouldn't
be in the synopsis since it is not needed for calling the function but
only for interpretation of the data returned by the function.  It also
fails to document the data format (which is an unusable mixture of struct
xfile and struct file).  It also fails to document the complete brokenness
of this function (the function now returns with an error without actually
reading any data).  This is harmless because the function is never used
in /usr/src.

Bruce