On Mon, Feb 06, 2012 at 02:37:29PM +0200, Kalle Valo wrote:
> On 02/06/2012 11:06 AM, Vasanthakumar Thiagarajan wrote:
> > On Mon, Feb 06, 2012 at 08:23:40AM +0200, Kalle Valo wrote:
> >> When debugging firmware issues it's not always enough to get
> >> the latest firmware logs, sometimes we need to get logs from a longer
> >> period. To make this possible, add a debugfs file named fwlog_block. When
> >> reading from this file ath6kl will send firmware logs whenever available
> >> and otherwise it will block and wait for new logs.
> >>
> >> Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxxxx>
>
> [...]
>
> >> + not_copied = copy_to_user(user_buf, buf, len);
> >> + if (not_copied != 0) {
> >> + ret_cnt = -EFAULT;
> >> + goto out;
> >> + }
> >> +
> >> + *ppos = *ppos + len;
> >
> > Why not to use simple_read_from_buffer()?, looks like it can also
> > takes care of len == 0 case in the following check.
> >
> > if (pos >= available || !count)
> > return 0;
> >
> > when available (len) is 0, pos = available with
> > ath6kl_fwlog_block_read().
>
> I actually used simple_read_from_buffer() first, but the problem is that
> it assumes that there's just one buffer from which the data is copied.
> But in this case there can be multiple buffers from which I copy data.
>
> Ok, that was a bit confusing, let's try to explain a bit differently :)
>
> If 'ppos > 0' (for example during the second function call)
> simple_read_from_buffer() will try to copy from 'user_buf + ppos' but I
> would want to copy from 'user_buf'.
I think you mean s/user_buf/buf. Are you not making sure that the
length of the data is not more than the requested one which is
passed to copy_to_user() so that read() is always called with
*ppos=0?. The following code seems to do that
while ((skb = __skb_dequeue(&ar->debug.fwlog_queue))) {
+ if (skb->len > count - len) {
+ /* not enough space, put skb back and leave
*/
+ __skb_queue_head(&ar->debug.fwlog_queue,
skb);
+ break;
+ }
May be my understanding is wrong.
Vasanth
--
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
