Hello Jiri and thanks a lot for the review ;-)
2009/1/4 Jiri Slaby <jirislaby@xxxxxxxxx>:
>> +/* debugfs: EEPROM stuff */
>> +
>> +/* EEPROM Header (common) */
>> +static ssize_t read_file_eeprom_header(struct file *file, char __user *user_buf,
>> + size_t count, loff_t *ppos)
>> +{
>> +
>> + struct ath5k_softc *sc = file->private_data;
>> + struct ath5k_hw *ah = sc->ah;
>> + struct ath5k_eeprom_info ee = ah->ah_capabilities.cap_eeprom;
>> + char buf[2000];
>
> Please don't use that much memory from stack. 2k is way too much. Note that
> you use yet another bunch of stack (next 3k here on 64-bit) by
> ath5k_eeprom_info and 32-bit x86 can still be configured with 4k stacks.
>
> Convert both of them to dynamically allocated buffers.
>
Why dynamically allocated buffers (can you point out some docs on
these please ?) ? The length of each file (what we print) is standard.
I understand that we use too much stack here but we can work this out
eg. by using a pointer to ath5k_eeprom_info or something like that. I
checked out user_buffer btw and it's 4K, sometimes it's not much for
calibration data (eg. on RF5111 that we have 10 points per pd gain
curve or RF2413+ that we can have multiple pd gain curves per
channel).
>> +/* EEPROM Header (per mode) */
>> +static unsigned int dump_calinfo_for_mode(struct ath5k_hw *ah, int mode,
>> + char __user *user_buf, size_t count, loff_t *ppos)
>> +{
>> + struct ath5k_eeprom_info ee = ah->ah_capabilities.cap_eeprom;
>> + unsigned int len = 0;
>> + char buf[2000];
>
> dtto
>
What does dtto mean ?
>> + int i;
> ...
>> + len = simple_read_from_buffer(user_buf, count, ppos, buf, len);
>
> simple_read_from_buffer can fail and returns negative errno in that case,
> change dump_calinfo_for_mode return type appropriately.
>
ACK
> Happy new year!
Thank you verry much, happy new year !! ;-)
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
--
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
