On Friday 10 September 2010 21:07:44 John Mahoney wrote:
> On Fri, Sep 10, 2010 at 2:39 PM, John Mahoney <jmahoney@xxxxxxxx> wrote:
> > On Fri, Sep 10, 2010 at 1:30 PM, fabio de francesco <fabio@xxxxxxxxxxx>
wrote:
> >> On Friday 10 September 2010 19:16:31 John Mahoney wrote:
> >>> On Fri, Sep 10, 2010 at 1:02 PM, John Mahoney <jmahoney@xxxxxxxx> wrote:
> >>> > On Fri, Sep 10, 2010 at 11:51 AM, fabio de francesco
> >>> > <fabio@xxxxxxxxxxx>
> >>
> >> wrote:
> >>> >> Hi all,
> >>> >>
> >>> >> I have modified a simple character device driver as an exercise from
> >>> >> the Cooperstein's Linux Device Drivers book.
> >>> >>
> >>> >> It seems to work fine except that when I "cat /dev/mycdrv" it
> >>> >> provides garbage.
> >>> >>
> >>> >> This is a trimmed down version of the code:
> >>> >>
> >>> >> #include <linux/module.h> /* for modules */
> >>> >> #include <linux/fs.h> /* file_operations */
> >>> >> #include <linux/uaccess.h> /* copy_(to,from)_user */
> >>> >> #include <linux/init.h> /* module_init, module_exit */
> >>> >> #include <linux/slab.h> /* kmalloc */
> >>> >> #include <linux/cdev.h> /* cdev utilities */
> >>> >>
> >>> >> #define MYDEV_NAME "mycdrv"
> >>> >> #define KBUF_SIZE (size_t)( PAGE_SIZE )
> >>> >>
> >>> >> static char *kbuf;
> >>> >> static dev_t first;
> >>> >> static unsigned int count = 1;
> >>> >> static int my_major = 700, my_minor = 0;
> >>> >> static struct cdev *my_cdev;
> >>> >> static int counter = 0;
> >>> >>
> >>> >> static int mycdrv_open (struct inode *inode, struct file *file)
> >>> >> {
> >>> >> printk( KERN_INFO " open #%d\n", ++counter );
> >>> >> kbuf = kmalloc (KBUF_SIZE, GFP_KERNEL);
> >>> >> memset( kbuf, '\0', KBUF_SIZE );
> >>> >
> >>> > First this should be
> >>> > memset( kbuf, '0', KBUF_SIZE );
> >>> >
> >>> > That will print the char 0 instead of the null char.
> >>> >
> >>> > Second try using "dd if=[dev] count=1" instead to read.
> >>> >
> >>> > --
> >>> > John
> >>>
> >>> If you actually want to fix it add the following check to the top of
> >>> mycdrv_read
> >>>
> >>> if (*ppos + kbuf > kbuf + KBUF_SIZE) {
> >>> printk (KERN_INFO "\n READING function, maxbytes=%d,
> >>> bytes_to_do=%d, lbuf=%d\n", maxbytes, bytes_to_do, lbuf);
> >>> return 0;
> >>> }
> >>>
> >>> --
> >>> John
> >>
> >> Hi John,
> >>
> >> I am sorry i am not able to understand how that check could fix the bug
> >> that certainly is in my code somewhere. Please do you mind to explain
> >> where the bug is located?
> >
> > You really should do something like this:
> > maxbytes = (kbuf + KBUF_SIZE) - (kbuf + *ppos);
> >
> > I am at work so I just scanned your code real quick, but basically you
> > were always reading a min of 4096 bytes and were reading past the end
> > of the buffer. You were returning always 4096 instead of how many
> > bytes were really left in the buffer.
> >
> > btw, you can do memset to null if you want.
>
> here is my version of the function:
>
> static ssize_t
> mycdrv_read (struct file *file, char __user * buf, size_t lbuf, loff_t *
> ppos) {
> int nbytes, maxbytes, bytes_to_do;
>
> maxbytes = (kbuf + KBUF_SIZE) - (kbuf + *ppos);
This is the same code as "maxbytes = KBUF_SIZE - *ppos;".
> bytes_to_do = lbuf <= maxbytes ? lbuf : maxbytes;
>
> nbytes = bytes_to_do - copy_to_user (buf, kbuf + *ppos, bytes_to_do);
> *ppos += nbytes;
>
> printk (KERN_INFO "\n READING function, nbytes=%d, pos=%d\n", nbytes,
> (int)*ppos);
> return nbytes;
> }
>
> --
> John
In what your function is different from mine?
fabio
--
To unsubscribe from this list: send an email with
"unsubscribe kernelnewbies" to ecartis@xxxxxxxxxxxx
Please read the FAQ at http://kernelnewbies.org/FAQ
