Generic List in C using void pointer

Question 1

You'll need to delegate type-aware operations to a separate function, then attach that function to your list via a function pointer. I've taken the liberty of changing your struct type definitions to something I've used before that I feel is a little more natural. Feel free to disagree.

struct Node {
  void *data;
  struct Node *prev;
  struct Node *next;
};

Unless you intend for the Item struct type to hold anything other than a void *, then get rid of it. Abstraction is a Good Thing, but there is such a thing as going overboard. Personally, I think it's cleaner to not create a bunch of typedefs, and I really don't like typedef'ing pointer types (pointer semantics are special and should not be hidden). Of course, YMMV.

struct List {
  struct Node *head;
  struct Node *tail;
  int (*cmp)( const void *, const void *);
};

I've modified your List type to contain your head and tail pointers, as well as a pointer to a comparison function. You can use this structure to create lists of any type; all you need to do is create a comparison function for that type and attach it to the list. For example, if you want your list to contain integers:

int compareInt( const void *lhs, const void *rhs )
{
  const int *llhs = (const int *) lhs;  
  const int *lhrs = (const int *) rhs;  

  if ( *llhs < *lrhs )                  
    return -1;
  else if ( *llhs > *lrhs )
    return 1;

  return 0;
}

The comparison function follows the same model as those used by qsort; it takes two parameters of const void * and returns an int value -- -1 if lhs < rhs, 1 if lhs > rhs, and 0 if lhs == rhs.

struct List intList = { NULL,  NULL, compareInt };

bool contains( const Item data, struct List *l ) 
{
  bool result = false;

  assert( l != NULL && l->cmp != NULL);

  struct Node *cur = l->head;

  while ( cur != NULL )
  {
    if ( l->cmp( cur->data, data ) == 0 )
    {
      result = true;
      break;
    }
    cur = cur->next;
  }

  return result;
}

So your function will walk through the list and compare values, returning true if it finds a match, false otherwise.

Now, this approach has a lot of drawbacks; it's complex, it's messy, it can be hard to follow, it can involve a lot of memory management, and you lose any pretense of type safety. There's nothing to stop you from associating the wrong function with a list, or mixing up types within the list. But, it does allow you to create "generic" containers that can handle any type, and you can add new types without having to hack the basic container logic. You only need to implement a new comparison function for each type.

Although, to be honest, you should implement not only comparison functions, but assignment, copy, display, and deallocation functions as well, attaching them to the list in the same way, along with a lightweight, type-aware interface for each type. It's more code, but it will save you heartburn in the long run.

Question 2

You should set isInList to false at the start and only mark it to true when you find a match. Then terminate the loop:

typedef struct Item
{
   void* data;
} Item;

typedef struct Node
{
   Item Item;
   struct Node* next;
   struct Node* previous;
} Node;

typedef Node* List;

bool is_element_of(Item Item, List *pointertolist)
{
   bool isInList = false;
   Node *scan = *pointertolist;
   while (scan != NULL && !isInList)
   {
      if ((scan->Item.data) == (Item.data))
      {
         printf("Match!\n");
         isInList = true;
      }
      scan = scan->next;
   }
   if(!isInList)
      printf("No Match!\n");
   return isInList;
}

Test Function:

void testit()
{
   Node n1;
   Node n2;
   Node n3;

   Item item;


   n1.Item.data = (void*)0x23;
   n2.Item.data = (void*)0x24;
   n3.Item.data = (void*)0x25;

   n1.next = &n2;
   n2.next = &n3;
   n3.next = NULL;

   List list = &n1;

   item.data = (void*)0x23;
   is_element_of(item, &list);

   item.data = (void*)0x24;
   is_element_of(item, &list);

   item.data = (void*)0x25;
   is_element_of(item, &list);

   item.data = (void*)0x26;
   is_element_of(item, &list);

}

Output Result:

Match!
Match!
Match!
No Match!

Question 3

You cannot de-reference a void* and compare the value because it could be of any size (ex: how would you compare a int and a short?

Without more knowledge of you data (yes, its always about the input data ;-), its hard to give a more definite answer. But here are two trails you may pursue...

[edit] I assume that your list may contain data of any type within the same list. People often use void * because they want to link different types together.

The short (bad) answer

you have to choose between comparing values, or comparing pointers. if you don't mind comparing only pointers then you could simply set your Item by giving it the address of your data:

node.Item.data =  &myval;

in which case you can see if you've already added this location in ram to your nodes. But this will not allow you to compare the same value in two different locations in your app. Thus if x and y in the following have the same value you wouldn't be able to compare that you have two nodes which point to 1.

x = 1;
y = 1;
node.Item.data = &x;
node2.Item.data = &y;

Furthermore, if you added any node which was allocated on the stack, you will quickly shovel yourself into a grave (as nodes may eventually refer to addresses which are not valid on the stack anymore)!

The longer (better) answer

When I have a generic accumulator system like this, instead of using a void *, I use a union.

enum {
    tp_int =    0x0001,
    tp_short =  0x0002,
    tp_vptr =   0x0004, // void pointer
    tp_sptr =   0x0008  // string pointer (whatever type you use 
                        // for your strings... ? char *)
    // ... add other types, including structs you may want to compare...
};

typedef struct Item {
    int type;   // <- essential!
    union data {
        int   i;
        short s;
        void *ptr; // all pointer types can use the same, unless you want to compare 
                   // compound values (a struct member?) where you'd then use the 
                   // tp_xxx to properly select which
                   // comparison to use ... as below.
    };
} Item;


typedef struct Node{
    Item Item; 
    struct Node* next; 
    struct Node* previous;
} Node;

typedef Node* List;

bool is_element_of(Item Item, List *pointertolist) {
    bool isinlist = false;
    Node *scan = *pointertolist; 
    while (scan->next != NULL) {
        //isinlist = false;
        if (scan->Item.type == Item.type){
            if (Item.type & (tp_vptr | tp_sptr)){
                // compare pointer types
                if ((scan->Item.ptr) == (Item.ptr)){
                    printf("pointer Match!");
                    isinlist = true;
                    break;
                }
            } else if (Item.type == tp_int){
                // compare integers (4 bytes?)
                if ((void *)(scan->Item.i) == (void *)(Item.i)){
                    printf("integer Match!");
                    isinlist = true;
                    break;
                }
            } else if (Item.type == tp_short){
                // compare shorts (2 bytes?)
                if ((scan->Item.s) == (Item.s)){
                    printf("short Match!");
                    isinlist = true;
                    break;
                }
            }
        } 
        scan = scan->next; 
    }

    return isinlist;
}

Note the above code may have one or two odd errors in it, I am not set up to compile it at the moment.

Here you can select which comparison to use. It also allows you to properly and safely use value types (like ints and shorts).

if you had more complex structs, you can easily compare them using their data instead of their pointers, so you could see if you had equivalent data.

You could even extend the above to check for similar strings, even if they have different memory locations. (when Item.type is tp_sptr :-)