[LCP]problem with put_user function

[Erik Mouw]

On Wed, Aug 29, 2001 at 09:37:48AM +0530, Srinath wrote:
> i am just learning programming in linux. i was just working with character
> device files.

Next time use a more on-topic mailing list, like the kernelnewbies
mailing list: kernelnewbies at nl.linux.org, web page at
http://www.kernelnewbies.org/, IRC channel #kernelnewbies on the
openprojects.net IRC network.

> i tried the following program on redhat 6.2 with kernel 2.2.14

Technically speaking this is not a program but a kernel module.

> i am getting this error
> unresolved symbol __put_user_X
> did i miss any #includes or #defines ??

You probably forgot to compile with -O2, so the macros in
include/asm/uaccess.h didn't get expanded. The correct way to compile a
kernel module is:

  gcc -O2 -Wall -D__KERNEL__ -DMODULE -I/usr/src/linux/include -c foo.c \
    -o foo.o

I'll add some comments to the code as well.

> #include<linux/kernel.h>
> #include<linux/module.h>

You're missing linux/init.h overe here.

> #include<linux/fs.h>
> #include<linux/wrapper.h>

You don't need linux/wrapper.h, that's only for backwards
compatibility.

> #include<asm/uaccess.h>
> 
> static char message[60];
> static char *messptr;
> static int dev_open_cnt=0;

Don't initialise global variables at 0, global variables are
automatically initialised at 0. Explicitly initialised global variables
live in the .data segment making the resulting binary (or module)
larger then necessary. Automatically initialised global variables live
in the .bss segment which is discarded from the binary; the variables
will be initialised at run time.

Getting rid of all zero initialised global variables made the kernel
allmost 100K smaller. (BTW, this trick not only works for kernel
modules, but also for userland programs).

> 
> static int dev_open(struct inode *inode, struct file *file)
> {
> 
>  dev_open_cnt++; 

Don't do the module counting yourself, but use MOD_INC_USE_COUNT;

>  sprintf(message,"Youve opened it the %dxx time",dev_open_cnt);
>  messptr=message;
>  
>  return 0;
> }
> 
> static void dev_release(struct inode *inode, struct file *file)
> {
>  dev_open_cnt--;

Same over here: use MOD_DEC_USE_COUNT;

> }
> 
> static ssize_t dev_read(struct file *file, char *buffer, size_t length,
>  loff_t *offset)
> {
>  int bytes_read=0;
> 
>  if(*messptr==0) return 0;
> 
>  while(length && *messptr)
>  {
>   put_user(*messptr++,buffer++);        /* is this the error zone ???*/
             ^^^^^^^^^^^^^^^^^^^
put_user() expects *addresses*, not *values*. Remove the * from the
messptr.

>   bytes_read++;
>   length--;
>  }

Don't invent your own version of memcpy(), but use copy_to_user() to
copy longer strings. You should also check the return value of
put_user() or copy_to_user(): they might fail because of an invalid
userland address in buffer.

Oh, and you forgot to do *offset += bytes_read;

>  return bytes_read;
> }
> 
> static ssize_t dev_write(struct inode *inode, struct file *file, const char *buffer,
>  int length)
> {
>  return -EINVAL;
> }

If you don't want the device to be writeable, don't provide a write
function. The kernel will automatically return an error if some process
wants to write the device.

> static int major;
> struct file_operations fops=
> {
>  NULL,
>  dev_read,
>  dev_write,
>  NULL,
>  NULL,
>  NULL,
>  NULL,
>  dev_open,
>  NULL,
>  dev_release
> };

This is completely unportable. When (yes, "when", not "if") struct
file_operations is changed, your module will break horribly. Use named
initialisers to initialise the structure:

struct file_operations fops = {
	read:		dev_read,
	open:		dev_open,
	release:	dev_release,
};

It makes the code portable between kernel versions, makes it a lot
harder to do it wrong, and makes the code much more readable.

> int init_module()
> {
>  major=module_register_chrdev(0,"chrdev",&fops);

Check the return value of module_register_chrdev(), it might fail when
you try to automatically get a device number.

>  printk("major %d \n",major); 
>  return 0;
> }
> 
> void cleanup_module()
> {
>  module_unregister_chrdev(major,"chrdev");
> }

Anyway, all this is explained in the book "Linux device drivers (2nd
edition)". You can download the book for free from:

  http://www.oreilly.com/catalog/linuxdrive2/

Or you can buy a hard copy from every good computer bookstore.


Erik

-- 
J.A.K. (Erik) Mouw, Information and Communication Theory Group, Department
of Electrical Engineering, Faculty of Information Technology and Systems,
Delft University of Technology, PO BOX 5031,  2600 GA Delft, The Netherlands
Phone: +31-15-2783635  Fax: +31-15-2781843  Email: J.A.K.Mouw at its.tudelft.nl
WWW: http://www-ict.its.tudelft.nl/~erik/