[head tinderbox] failure on amd64/amd64

[Eygene Ryabinkin]

Hiroki, good day.

Mon, Jun 08, 2009 at 04:53:25PM +0900, Hiroki Sato wrote:
>  Gr, certainly this looks strange.  I meant the attached patch.
>  Thanks for pointing out it.

No problems.

But still:

> Index: if_gif.c
> ===================================================================
> --- if_gif.c	(revision 193673)
> +++ if_gif.c	(working copy)
> @@ -914,10 +914,10 @@
>  	case GIFSOPTS:
>  		if ((error = priv_check(curthread, PRIV_NET_GIF)) != 0)
>  			break;
> -		if ((error = copyin(&options, &sc->gif_options,
> -				sizeof(sc->gif_options)))) {
> +		if ((error = copyin(ifr->ifr_data, &options,
> +				sizeof(options)))) {
>  			if ((options | GIF_FULLOPTS) == GIF_FULLOPTS)
> -				ifr->ifr_data = (caddr_t)options;
> +				sc->gif_options = options;
>  			else
>  				error = EINVAL;
>  		}

Do you intend to set sc->gif_options only for the case of failed
copyin()?  This looks a bit strange to me too, because
 1. in this case 'options' will have undeterminate contents;
 2. I thought that 'set options' should set options if it is
    permitted.
Though there could be some logics behing this -- don't know, but
may be the negation operator was lost before '(error = copyin(...))' --
this is most adequate description of check for GIF_FULLOPTS.

By the way, it will be great if new sysctls and their options will be
documented somewhere, perhaps in the gif(4) itself.
-- 
Eygene
 _                ___       _.--.   #
 \`.|\..----...-'`   `-._.-'_.-'`   #  Remember that it is hard
 /  ' `         ,       __.--'      #  to read the on-line manual
 )/' _/     \   `-_,   /            #  while single-stepping the kernel.
 `-'" `"\_  ,_.-;_.-\_ ',  fsc/as   #
     _.-'_./   {_.'   ; /           #    -- FreeBSD Developers handbook
    {_.-``-'         {_/            #