Messages in this thread |  | | Subject | Re: select() support for NR_OPEN>256 | Date | Wed, 05 Feb 1997 21:18:45 +0100 | From | Andi Kleen <> |
| |
In message <199702051923.UAA08601@sunsite.ms.mff.cuni.cz>you write: >> - * We do a VERIFY_WRITE here even though we are only reading this time: >> - * we'll write to it eventually.. >> - * >> * Use "int" accesses to let user-mode fd_set's be int-aligned. >> */ >> static int __get_fd_set(unsigned long nr, int * fs_pointer, int * fdset) >> @@ -209,16 +205,14 @@ >> /* round up nr to nearest "int" */ >> nr = (nr + 8*sizeof(int)-1) / (8*sizeof(int)); >> if (fs_pointer) { >> - int error = verify_area(VERIFY_WRITE,fs_pointer,nr*sizeof(int)) >; >> - if (!error) { >> - while (nr) { >> - get_user(*fdset, fs_pointer); >> - nr--; >> - fs_pointer++; >> - fdset++; >> - } >> + while (nr) { >> + if (get_user(*fdset, fs_pointer)) >> + return -EFAULT; >> + nr--; >> + fs_pointer++; >> + fdset++; >> } >> - return error; >> + return 0; > >Isn't it better to leave there the verify_area and just use __get_user >instead of get_user? This will generate much nicer code (at least on the >sparc)... > >Like: > > int error = verify_area(VERIFY_WRITE,fs_pointer,nr*sizeof(int)); > if (!error) { > while (nr) { > if (__get_user(*fdset, fs_pointer)) > return -EFAULT; > nr--; > fs_pointer++; > fdset++; > } > } > return error;
Yup, you're right. My original code replaced the whole function with a copy_from_user() but I got some strange errors (I suspected a gcc problem) so I reverted back to this simple-minded implementation. The copy_from_user() macro also expanded to unneeded byte-access code.
I'll change my code to keep the verify_area().
-Andi
|  |