Waring about pointers

K

kyle.tk

I am having some (compiler) problems with the following code. The
compiler complains of "warning: assignment makes pointer from integer
without a cast" at the lines I indicated. But it still compiles and
works as expected. So what is the proper way to write this.


void test(void){
printk("test\n");
}

/* In main */
register_isr(29,test);

/* In isr.c */
int register_isr(unsigned int vector, void (*isr)(void)){
volatile unsigned long *vaddr; /* Vector Address */
vaddr = (volatile unsigned long) EVT_BASE + (vector * 4); /*
warings here */
*vaddr = isr; /* and here */
return 0;
}

So whats wrong with this?

-kyle
 
K

Keith Thompson

kyle.tk said:
I am having some (compiler) problems with the following code. The
compiler complains of "warning: assignment makes pointer from integer
without a cast" at the lines I indicated. But it still compiles and
works as expected. So what is the proper way to write this.


void test(void){
printk("test\n");
}

/* In main */
register_isr(29,test);

/* In isr.c */
int register_isr(unsigned int vector, void (*isr)(void)){
volatile unsigned long *vaddr; /* Vector Address */
vaddr = (volatile unsigned long) EVT_BASE + (vector * 4); /*
warings here */
*vaddr = isr; /* and here */
return 0;
}

So whats wrong with this?

vaddr is a pointer (of type volatile unsigned long*). The value you
assign to it is an integer (of type volatile unsigned long).

In the second assignment, "*vaddr = isr;", isr is a pointer to a
function; *vaddr is an unsigned long.

You're obviously doing some tricky low-lever stuff here. Without more
information (we don't know what EVT_BASE is, for example), it's hard
to tell what you *should* be doing.
 
F

Flash Gordon

kyle.tk said:
I am having some (compiler) problems with the following code. The
compiler complains of "warning: assignment makes pointer from integer
without a cast" at the lines I indicated. But it still compiles and
works as expected. So what is the proper way to write this.

void test(void){
printk("test\n");
}

/* In main */
register_isr(29,test);

/* In isr.c */
int register_isr(unsigned int vector, void (*isr)(void)){
volatile unsigned long *vaddr; /* Vector Address */

Here you declare vaddr as a pointer.
vaddr = (volatile unsigned long) EVT_BASE + (vector * 4); /*
warings here */

Here you assign an integer to it. Integers and pointers are *not* the
same thing, so the compiler is quite correct to complain.

Not knowing the definition of EVT_BASE is it impossible to say what the
correct solution to it is. However, I would guess that what you want to
do is loose the cast and ensure that EVT_BASE is of the correct type.
*vaddr = isr; /* and here */

Here you try to assign a pointer to a function to the location that
vaddr points to but vaddr is defined as pointing to an integer
(specifically an unsigned long) not a function pointer.
return 0;
}

So whats wrong with this?

The problem seems to be you thinking pointers and integers are the same
thing. They most definitely are *not* the same.

If you post a compilable minimal example (which would at least include
the definition of EVT_BASE and preferable some sample code calling this
function) together with what you think you are trying to do we can help
better.

I'm guessing that EVT_BASE is the start of an interrupt vector table
(which is OT here) and this function is trying to assign functions to
specific location.

I think what you actually want is something like the following untested
code:
typedef void (*isr_t)(void);
#define EVT ((volatile isr_t*)0x1234)

void register_isr(unsigned int vector, isr_t isr)
{
EVT[vector] = isr;
}

Another possible definition for EVT is:
extern volatile isr_t EVT[NO_OF_ISRS];
Then you can possibly use some linker magic (which is OT here) to place
EVT at the correct location.

Note the use of the typedef for the function pointer to make writing the
rest of it simpler. Also note that apart from casting the address of the
interrupt vector table to a pointer I've got rid of the need to cast
anything. In general, casts are bad. However, if I'm correct in my
assumptions of what you are trying to do, this is an instance where you
need to convert an integer to a pointer, something which gives an
implementation defined result at best and undefined behaviour at worst.

Please note that you are skirting the edges of topicality. How your
interrupt vector table works in not topical, but given that it is an
array of pointers to functions at an address that you know we can
discuss ways you can write to it.
--
Flash Gordon
Living in interesting times.
Web site - http://home.flash-gordon.me.uk/
comp.lang.c posting guidelines and intro -
http://clc-wiki.net/wiki/Intro_to_clc
 
K

kp.discuss

Hi,

kyle.tk said:
/* In isr.c */
int register_isr(unsigned int vector, void (*isr)(void)){
volatile unsigned long *vaddr; /* Vector Address */
vaddr = (volatile unsigned long) EVT_BASE + (vector * 4); /*
warings here */

1) There could be 2 errors, one logical and one cast. The statement
could be one of the following (Assuming that the isr address should be
located at 29*4 bytes from EVT_BASE)..
- vaddr = (volatile unsigned long*)EVT_BASE + vector ;
- vaddr = (volatile unsigned long*)(EVT_BASE + (vector * 4));

2) Do you think that the volatile declaration is necessary in this
context? I dont see the cause for that memory location to be volatile.
*vaddr = isr; /* and here */
return 0;

3) The above statement also has the cast issues. Either declare the
"vaddr" to be a "function pointer type" or type-cast the "isr" to
"vaddr" type. This could eliminate the warnings.
 
K

kp.discuss

Hi,

kyle.tk said:
/* In isr.c */
int register_isr(unsigned int vector, void (*isr)(void)){
volatile unsigned long *vaddr; /* Vector Address */
vaddr = (volatile unsigned long) EVT_BASE + (vector * 4); /*
warings here */

1) There could be 2 errors, one logical and one cast. The statement
could be one of the following (Assuming that the isr address should be
located at 29*4 bytes from EVT_BASE)..
- vaddr = (volatile unsigned long*)EVT_BASE + vector ;
- vaddr = (volatile unsigned long*)(EVT_BASE + (vector * 4));

2) Do you think that the volatile declaration is necessary in this
context? I dont see the cause for that memory location to be volatile.
*vaddr = isr; /* and here */
return 0;

3) The above statement also has the cast issues. Either declare the
"vaddr" to be a "function pointer type" or type-cast the "isr" to
"vaddr" type. This could eliminate the warnings.
 
Q

Q

to follow up on what Keith said, it is hard to give accurate advice not
fully understanding all of the variables

however, in the statements where u have:
vaddr = (volatile unsigned long) EVT_BASE + (vector * 4);

when assigning a value to a pointer, it expects a pointer (address) in
return, UNLESS you dereference it first (i.e. *vaddr = (. . .))

since your placing a value in the space pointed to by vaddr, try using
*vaddr= instead
> *vaddr = isr;

"isr" runs and returns no value (void) to a variable that expects a
"volatile unsigned long". it should be safe to call isr as follows:
isr(); or (*isr) ();

without using the *vaddr=
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,756
Messages
2,569,535
Members
45,008
Latest member
obedient dusk

Latest Threads

Top