Philippe De Muyter wrote: > On Mon, May 24, 2010 at 06:26:08PM +0100, Jamie Lokier wrote: > > Philippe De Muyter wrote: > > > On Mon, May 24, 2010 at 04:59:18PM +0100, David Howells wrote: > > > > Philippe De Muyter <p...@macqel.be> wrote: > > > > > > > > > > +#else > > > > > > +#define TASK_SIZE (0xFFFFFFFFUL) > > > > > > +#endif > > > > > > > > > > Because of do_getname() : > > > > > > > > > > len = TASK_SIZE - (unsigned long) filename; > > > > > > > > > > we should rather have > > > > > > > > > > #define TASK_SIZE (0x100000000ull) > > > > > > > > Do you guarantee that will work everywhere on a 32-bit system, though? > > > > > > > > Note that it also makes things slower as gcc has to start using 64-bit > > > > arithmetic where it could otherwise use 32-bit arithmetic. > > > > > > Except if gcc notices that this simplifies to > > > > > > len = (unsigned long)(-filename); > > > > > > I don't know if it does. > > > > It did simplify on the x86 (gcc 4.4) and ARM (gcc 3.4.3) tests I just did. > > Also the comparison "addr < TASK_SIZE" simplified to "1". > > > > However I can imagine some logic like this going awry with that > > definition: > > > > if (addr >= TASK_SIZE || len > TASK_SIZE - addr) > > return -EINVAL; > > > > Think about the case addr == 0. > > It would give ( 0 || len > (unsigned long)(-addr)) > I don't see a problem there.
The problem is it would return -EINVAL for any non-zero value of len, which isn't what the code looks like it's meant to do. (By luck it would be ok because rejecting NULL addr is probably good anyway. But that might not be true for every check of that form.) -- Jamie _______________________________________________ uClinux-dev mailing list uClinux-dev@uclinux.org http://mailman.uclinux.org/mailman/listinfo/uclinux-dev This message was resent by uclinux-dev@uclinux.org To unsubscribe see: http://mailman.uclinux.org/mailman/options/uclinux-dev
