Date: Tue, 12 Nov 2013 14:31:35 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Sergey Kandaurov <pluknet@gmail.com> Cc: "freebsd-virtualization@freebsd.org" <freebsd-virtualization@freebsd.org> Subject: Re: Running `sysctl -a` causes crash (Hyper-V) Message-ID: <20131112133135.GB29757@dft-labs.eu> In-Reply-To: <CAE-mSOJFGos7Y-QvuskPDK9=Jv8CLqGPKi8Gck=Uj2mmy87Y0Q@mail.gmail.com> References: <CAAoTqfvrdKH6BC=KrpWLgBKDjx402oTt_bp1TnV_Z3FC1if88A@mail.gmail.com> <CAE-mSOLKgk_h10mUPLuMXHgPfYJMEog_FFdyZvEoAOXMh=rbfg@mail.gmail.com> <CAAoTqfu9NFuVFYnLFo%2BLG7_kXd%2BAaxEPTm4nCKETzWvje7=fyA@mail.gmail.com> <CAE-mSOLeMHkh7gKLZPYcyu3GdJKqN4hAMBgEf5TwMUxB4nQ7zA@mail.gmail.com> <CAAoTqftAbsDNUiCv4QRnNfamf61dPhJ%2BWAzgfVVL4QMe9qHxcA@mail.gmail.com> <CAE-mSOLszJLfHBgmSTaUH5%2B5dTe23XLUFbp-nhfPpz2UVWMCJQ@mail.gmail.com> <CAAoTqfu716D-ifL-GXurxveZOKahLpndAivBWWt6ENHOJxXaaA@mail.gmail.com> <52814D25.1050603@freebsd.org> <20131112030154.GA29757@dft-labs.eu> <CAE-mSOJFGos7Y-QvuskPDK9=Jv8CLqGPKi8Gck=Uj2mmy87Y0Q@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Nov 12, 2013 at 03:33:45PM +0400, Sergey Kandaurov wrote:
> On 12 November 2013 07:01, Mateusz Guzik <mjguzik@gmail.com> wrote:
> > On Mon, Nov 11, 2013 at 01:33:25PM -0800, Julian Elischer wrote:
> >> On 11/11/13, 7:49 AM, Pavel Timofeev wrote:
> >> >Great, it helped! Thanks a lot!
> >>
> >> the suggested change suggests fragile code. It should be able to
> >> handle unexpected input..
> >
> > Indeed, I already suggested Sergey placing a compile time assertion to
> > make sure vm_guest range is covered.
>
> I have the following change for this.
> Btw, the last element in vm_guest_sysctl_names is NULL, is it really needed
> for something? Unlike vm_bnames/vm_pnames, it seems not.
>
No idea, I would leave it be.
> Add CTASSERT to handle enum VM_GUEST and vm_guest_sysctl_names cooperation.
>
> Index: sys/sys/systm.h
> ===================================================================
> --- sys/sys/systm.h (revision 258036)
> +++ sys/sys/systm.h (working copy)
> @@ -72,7 +72,8 @@ extern int vm_guest; /* Running as virtual machin
> * ever implemented (e.g. vendor-specific paravirtualization features).
> * Keep in sync with vm_guest_sysctl_names[].
> */
> -enum VM_GUEST { VM_GUEST_NO = 0, VM_GUEST_VM, VM_GUEST_XEN, VM_GUEST_HV };
> +enum VM_GUEST { VM_GUEST_NO = 0, VM_GUEST_VM, VM_GUEST_XEN, VM_GUEST_HV,
> + VM_LAST };
>
> #if defined(WITNESS) || defined(INVARIANTS)
> void kassert_panic(const char *fmt, ...) __printflike(1, 2);
> Index: sys/kern/subr_param.c
> ===================================================================
> --- sys/kern/subr_param.c (revision 258036)
> +++ sys/kern/subr_param.c (working copy)
> @@ -156,6 +156,7 @@ static const char *const vm_guest_sysctl_names[] =
> "hv",
> NULL
> };
> +CTASSERT(nitems(vm_guest_sysctl_names) - 1 == VM_LAST);
>
> #ifndef XEN
> static const char *const vm_bnames[] = {
>
I can't test it right now, I presume it works :)
With that in mind looks good to me, thanks!
--
Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131112133135.GB29757>