tags:

views:

184

answers:

5

My professor defined this in the .h file

void list_map(INTLIST* list, void (*f)(void *)); /*Applies a function to each element of the list */

I wrote the function like this:

 void list_map(INTLIST* list, void (*f)(void *))
  {
   INTLIST* pTemp=NULL;

   if (list == NULL)
    {
      //list is empty
    }
   else
      {
       for(pTemp=list; pTemp->next!=NULL; pTemp=pTemp->next)
          {
             f(pTemp); //f is a function pointer we call list map from main like list_map(lst, list_sort)
          }    
      }
  }

I call it in main like this:

  list_map(aList[i], (void*)list_sort);

In windows environment, no complaints, but I have to run this in a Linux environment. I'm using a makefile to compile all of the code and I get this warning and error:

c++ -O2 -c main.c main.c: In function ‘int main(int, char*)’: main.c:53: warning: deprecated conversion from string constant to ‘char*’ main.c:123: error: invalid conversion from ‘void ()(INTLIST)’ to ‘void ()(void)’ main.c:123: error: initializing argument 2 of ‘void list_map(INTLIST*, void ()(void))’ make: * [main.o] Error 1*

Can somebody help with the error first and then maybe with the warning?

Edit Portion:

Someone asked for the list_sort function, here it is:

 void list_sort(INTLIST* list)
 {
  INTLIST* pTemp=NULL;
  INTLIST* pTemp2=NULL;

  pTemp=list;          //temp pointers to compare node values
  pTemp2=list;

  if (pTemp->next !=NULL)     //move to second node
   {
      pTemp2=pTemp2->next;
   }

  while(pTemp2 != NULL)
   {   
       //we implement a selection sort 
       //check if incoming node->datum with each node in the list
       //swap values if <
      if (pTemp2->datum < pTemp->datum)
         {
         //swap the values
         int temp = pTemp->datum;
         pTemp->datum = pTemp2->datum;
         pTemp2->datum = temp;
         }
         //advance the pointer
      pTemp2=pTemp2->next;
   }
 }
+1  A: 

And if you cast your callback to the proper function type?

list_map(aList[i], (void (*)(void*))list_sort);
zneak
"If what you get is not a duck, punch it until you get what you want" - I am sorry but this is a bad answer to give to a beginner...
LiraNuna
@LiraNuna: It's a homework and he can't change the prototype of list_map. I'm afraid his teacher wants him to punch it until it quacks like a duck.
zneak
this will make the warning go away, but now it'll invoke undefined behaviour when the function gets called within `list_map()`
Christoph
+2  A: 

Well,

void list_sort(INTLIST* list)

has the wrong signature to be passed as the second argument of

void list_map(INTLIST* list, void (*f)(void *))
dmckee
Correct answer.
Alex
Been trying to follow this stuff as I too am learning C...but I guess my question is it looks like list_map's void (*f) )(void*) is that just a function pointer? What could he change void list_sort to become? void list_sort(void* list) ?
JonH
A: 

In your function list_sort, the parameter is INTLIST *list.

list_map(aList[i], (void*)list_sort);

By looking at the header, the function prototype is a function pointer that has a parameter which is of type void *

void list_map(INTLIST* list, void (*f)(void *))
                                      ^^^^^^^^

The function pointer *f has to match up the signature, hence the conflict and the warning generated by your compiler. Since *f points to the list_sort, the method signature does not match up.

It would work if your function prototype has this instead

void list_map(INTLIST* list, void (*f)(INTLIST *))

Hope this helps, Best regards, Tom.

tommieb75
Cannot change the list_map prototype that was defined by the professor and we are not allowed to change those prototypes.
+1  A: 
Alok
These are the things I am looking for very well written details, I could care less about the answer I am looking to understand the problem to fully utilize and learn the language. Alok +1 plus accept for this thorough answer. You could write a book and I'd buy it.
+2  A: 

A simple cast of list_sort() is enough to make the warning go away, but it's not enough to make it actually work:

The C-standard doesn't guarantee that an INTLIST * and a void * have compatible representations, ie void (INTLIST *) and void (void *) are distinct, incompatible types. When list_map() invokes list_sort() through your void (*f)(void *) argument, C99 section 6.3.2.3, §8 applies:

If a converted pointer is used to call a function whose type is not compatible with the pointed-to type, the behavior is undefined.

To make it standard compliant, you have to write a wrapper function for list_sort():

void list_sort_wrapper(void *list)
{
    list_sort(list);
}

and use that as argument to your call:

list_map(aList[i], list_sort_wrapper);

Also, if list_sort() is implemented correctly (didn't check the algorithm), it already walks the list, ie invoking it for each node doesn't make any sense at all.

edit:

Ok, list_sort() doesn't actually sort the whole list - that can be achieved via

list_map(list, list_sort_wrapper);

The naming scheme is a serious WTF - if the function doesn't sort the list, call it list_sort_step() or list_select_head() or something, but please not list_sort().

Christoph
list_map was a mapping function that takes a function pointer to list_sort() so as long as you call list_map you end up sorting the entire list, even though list_sort is doing this element by element. I know it looks strange, but professors are strange people who really can't code.
+1 also a good explanation.