Linux Device Driver - what's wrong with my device_read()?

Question

EDIT:

If myinc_value is an N-digit integer, I get the first N/2 bytes of the msg buffer back in user space. So if myinc_value = 48, cat /dev/my_inc yields 4. If myinc_value = 489324, cat /dev/my_inc yields 489.

This behavior persists even after I added the following code directly after the while loop, to nul-terminate the user-space buffer:

        if(put_user('\0', buffer++))
            return -EFAULT;

Here is some output from dmesg:

[54471.381170] my_inc opened with initial value 489324 = 489324.
[54471.381177] my_inc device_read() called with value 489325 and msg 489324.
[54471.381179] my_inc device_read() read 4.
[54471.381182] my_inc device_read() read 8.
[54471.381183] my_inc device_read() read 9.
[54471.381184] my_inc device_read() read 3.
[54471.381185] my_inc device_read() read 2.
[54471.381186] my_inc device_read() read 5.my_inc device_read() returning 7.
[54471.381192] my_inc device_read() called with value 489325 and msg 489325.

And from the shell:

root@rbst:/home/rob/myinc_mod# cat /dev/my_inc
489root@rbst:/home/rob/myinc_mod#

ORIGINAL POST:

My device /dev/my_inc is meant to take a positive integer N represented as an ascii string, and store it. Any read from /dev/my_inc will produce the ascii string representation of N + 1.

The problem is that when I cat /dev/my_inc, I only get the first byte of myinc_value output to my shell, even though I have bytes_read == 2 at the end of my loop.

/* Read from the device 
 *********************/
static ssize_t device_read(struct file *filp, char *buffer, size_t length, loff_t *offset)
{
    char c;
    int bytes_read  = 0;
    int value   = myinc_value + 1;

    printk(KERN_INFO "my_inc device_read() called with value %d and msg %s.\n", value, msg);

    /* No bytes read if pointer to 0 */
    if (*msg_ptr == 0) 
        return 0;

    /* Put the incremented value in msg */
    snprintf(msg, MAX_LENGTH, "%d", value);

    /* Put bytes from msg into user space buffer */
    while (length && *msg_ptr) 
    {
        c = *(msg_ptr++);
        printk(KERN_INFO "%s device_read() read %c.", DEV_NAME, c);

        if(put_user(c, buffer++))
            return -EFAULT;

        length--;
        bytes_read++;
    }

    /* Nul-terminate the user-space buffer and include NUL in bytes read */
    if(put_user('\0', buffer++))
            return -EFAULT;

    bytes_read++;
    printk("my_inc device_read() returning %d.\n", bytes_read);

    /* Return bytes copied */
    return bytes_read;
}

Answer 1

The reason it only shows 1 byte is because you are incrementing the msg_ptr before setting it equal to c. It needs to be c = *msg_ptr++; or c = *msg_ptr; msg_ptr++; so that the increment happens after the assignment

Answer 2

It may be that put_user() is defined as a macro so that the post increment operator in

if(put_user(c, buffer++))

is screwing up - though I don't see how it explains what you are seeing.

Anyway it would be more convenient and more efficient to use copy_to_user() to copy the whole msg.