Re: [PATCH] fix grub-setup on kfreebsd by adding 0x10 to the sysctl ("kern.geom.debugflags") flags

[Robert Millan <rmh@aybabtu.com>]

On Mon, Apr 13, 2009 at 05:11:04PM +0200, Felix Zielcke wrote:
> > Btw you can avoid the two subsequent calls if sysctl_flags == sysctl_oldflags.
> 
> Right and actually it can be even avoided if 0x10 is already set.

It's the same thing (if and only if 0x10 was set, old and new flags will
be the same).  I'm not sure which check is cheaper, but it doesn't matter
much anyway.

> Is it correct that I used now `[__FreeBSD__]: Likewise.' in the
> Changelog?
> The GCS unfortunately doestn't tell anything about the case that a
> change applies to 2 or more marcros and I couldn't find an example in
> the existing Changelog for this.

How about [__FreeBSD__ || __FreeBSD_kernel__] ?

> +  if (sysctlbyname ("kern.geom.debugflags", &sysctl_oldflags, &sysctl_size, NULL, 0))
> +    grub_util_error ("cannot get current flags of sysctl kern.geom.debugflags");

I'd just return grub_error instead.  Otherwise we abort the program even if
failure to read a drive is not critical (e.g. lvm.mod scannning all drives,
grub-emu, etc).

> +  if (! sysctl_oldflags & 0x10 && sysctlbyname ("kern.geom.debugflags", NULL , 0, &sysctl_flags, sysctl_size))
> +    grub_util_error ("cannot set flags of sysctl kern.geom.debugflags");

Just a matter of taste, I'd suggest nested ifs to make it more readable.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."