bruno randolf <bruno@xxxxxxxxxxxxx> writes: > On Monday 21 January 2008 15:44:46 Kalle Valo wrote: >> > + char buf[20]; >> > + >> > + if (copy_from_user(buf, userbuf, min_t(size_t, count, 20))) >> > + return -EFAULT; >> >> How about let the compiler count the size of buf? Like this: >> >> if (copy_from_user(buf, userbuf, min_t(size_t, count, sizeof(buf)))) >> >> That way the size of buf is only in one place, and we might avoid a >> bug whenever someone changes the size of buf. > > good idea. that way i can also get rid of min_t(size_t,...) and use min(). Even better. >> > +static ssize_t read_file_debug(struct file *file, char __user *user_buf, >> > + size_t count, loff_t *ppos) >> > +{ >> > + struct ath5k_softc *sc = file->private_data; >> > + char buf[1000]; >> >> To my eyes allocating 1000 bytes from stack is too large. > > what about 700? ;) > the output currently is 626 byte. Hehe :) I have no idea what's the preferred limit here, so I cannot you give a definitive answer. But I would guess that 700 byte allocation is still sensible with 4k stacks. But this is for a debug interface so most likely this doesn't matter that much. -- Kalle Valo - To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html