Re: simple character device driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

--
John

--
To unsubscribe from this list: send an email with
"unsubscribe kernelnewbies" to ecartis@xxxxxxxxxxxx
Please read the FAQ at http://kernelnewbies.org/FAQ




[Index of Archives]     [Newbies FAQ]     [Linux Kernel Mentors]     [Linux Kernel Development]     [IETF Annouce]     [Git]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux SCSI]     [Linux ACPI]
  Powered by Linux