From owner-freebsd-amd64@FreeBSD.ORG Mon Jun 8 08:14:46 2009 Return-Path: Delivered-To: amd64@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 248421065676; Mon, 8 Jun 2009 08:14:46 +0000 (UTC) (envelope-from rea-fbsd@codelabs.ru) Received: from 0.mx.codelabs.ru (0.mx.codelabs.ru [144.206.177.45]) by mx1.freebsd.org (Postfix) with ESMTP id CA1FE8FC08; Mon, 8 Jun 2009 08:14:45 +0000 (UTC) (envelope-from rea-fbsd@codelabs.ru) DomainKey-Signature: a=rsa-sha1; q=dns; c=simple; s=one; d=codelabs.ru; h=Received:Date:From:To:Cc:Subject:Message-ID:Reply-To:References:MIME-Version:Content-Type:Content-Disposition:In-Reply-To:Sender; b=I9+mE8BUb4ayDeSdLbuGlpC0et4dY8iYtDo/jKlTdJrNxphAXwrpDz12RbFb34Jk2s7aUY2ULyKZ+ZOoC2pPNlG0qLSXubZyLBN0GstBkCZVYx9mlzq1ulIgg4kOc5sPMh2Soo7wlDWhcjP2/BadM6xh2Ja8RQpQzlMuW6aaEI4=; Received: from void.codelabs.ru (void.codelabs.ru [144.206.177.25]) by 0.mx.codelabs.ru with esmtpsa (TLSv1:AES256-SHA:256) id 1MDa0C-000JoZ-UL; Mon, 08 Jun 2009 12:14:45 +0400 Date: Mon, 8 Jun 2009 12:14:42 +0400 From: Eygene Ryabinkin To: Hiroki Sato Message-ID: References: <20090608025715.499087302F@freebsd-current.sentex.ca> <8LPG99US2/4EsGlonyfMSkDb40o@XX1fo6zQUfC4h0jjRC6IBz3oNH4> <20090608.165325.225640915.hrs@allbsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090608.165325.225640915.hrs@allbsd.org> Sender: rea-fbsd@codelabs.ru X-Mailman-Approved-At: Mon, 08 Jun 2009 11:35:02 +0000 Cc: amd64@FreeBSD.org, current@FreeBSD.org Subject: Re: [head tinderbox] failure on amd64/amd64 X-BeenThere: freebsd-amd64@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: rea-fbsd@codelabs.ru List-Id: Porting FreeBSD to the AMD64 platform List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Jun 2009 08:14:46 -0000 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 {_.-``-' {_/ #