re: CVS commit: src

[Paul Goyette <paul%whooppee.com@localhost>]

On Fri, 3 Nov 2017, matthew green wrote:

> thanks for working on this.
>
> > * Update the data structures used for exporting kernel history data to
> >   include a version number as well as the length of history arguments.
>
> so, old vmstat breaks for no reason?  nothing else changed, right?

Unfortunately, yes, old vmstat breaks.  But other stuff _did_ change, 
notably the change that all four of the printf-args are explicitly cast 
to uintmax_t before being stored in the kernel and before being 
"exported" to userland.

It was because of this breakage that I introduced a version-and-size 
check, to _cleanly_ detect any such mismatch in the future.

Note that vmstat was already quite broken on architectures such as arm, 
where passing a 64-bit value to printf(3) was processed as two separate 
32-bit args.

> > * All [2] existing users of kernhist(9) have had instances of "%s" or
> >   "%c" format strings replaced with numeric formats; several instances
> >   of mis-match between format string and argument list have been fixed.
>
> i wonder if we could have an audit mode.  it would actually
> pass the arguments to a printf()-like function so the compiler
> would do checking.

That would have made the task much simpler.

> > * vmstat(1) now checks the version and argument length included in the
> >   data exported via sysctl(9) and exits if they do not match the values
> >   with which vmstat was built.
>
> that's not how we do sysctl.  if we were just going to do this
> we may as well just do kvm.  sysctl should be ABI stable.

As far as I know, the sysctl we're discussing is newly-introduced for 
NetBSD-8, so it has _never_ been part of any release.  The version and 
length checks can easily be removed - they were intended only for the 
convenience of those who follow -current.

> please revert all the ABI changes.  they're not necessary and
> they're just not how sysctl is done.

Well, since "all the ABI changes" includes the sizes of the members of 
the data structures, that would mean reverting _all_ the recent changes, 
and going back to the broken state (at least for some architectures).
(It isn't clear - to me, at least - if this "please revert" also refers 
to the original sysctl-ification that was done so many months ago.)

If you can be more explicit and detailed about exactly what you want 
done, I will look into it further.  Please address each of:

	- version-and-length check
	- other changes to the data structures, resulting in the
	  conversion of printf()'s arguments to uintmax_t in both
	  in-kernel and export structures (as opposed to ulong and
	  uint64_t respectively)
	- the whole sysctl-ification





+------------------+--------------------------+----------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:          |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+------------------+--------------------------+----------------------------+