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) > _at__at_ -914,10 +914,10 _at__at_ > 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 {_.-``-' {_/ #Received on Mon Jun 08 2009 - 06:14:46 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:49 UTC