Warnings with Pointer In Linked List

Hello, I just found this board due to some unresolved trouble I've been having with a program I'm writing. When I compile (on gcc) I receive this warning.

b_poplist.c:48: warning: assignment from incompatible pointer type

If anyone knows why this warning comes up and could assist me in correcting it, it would be much appreciated.

I have tried to cast the right side as (b_list*) but that heeds no different results.
ie: cb_list->next = (b_list*)hb_list

Here is my code for b_poplist.c

Code:

#include "b_list.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <bluetooth/bluetooth.h>
#include <bluetooth/hci.h>
#include <bluetooth/hci_lib.h>
#include <unistd.h>

b_list *b_poplist(void)
{
	inquiry_info *ii = NULL;
        int max_rsp, num_rsp;
        int dev_id, sock, len, flags;
        int i;
        char addr[19] = { 0 };
        char name[248] = { 0 };

        dev_id = hci_get_route(NULL);
        sock = hci_open_dev( dev_id );
        if (dev_id < 0 || sock < 0) {
            perror("opening socket");
            exit(1);
        }

        len  = 8;
        max_rsp = 255;
        flags = IREQ_CACHE_FLUSH;
        ii = (inquiry_info*)malloc(max_rsp * sizeof(inquiry_info));
    
        num_rsp = hci_inquiry(dev_id, len, max_rsp, NULL, &ii, flags);
        if( num_rsp < 0 ) perror("hci_inquiry");
	
	b_list *hb_list, *cb_list;
	hb_list = NULL;
	
	for (i = 0; i < num_rsp; i++) {
		ba2str(&(ii+i)->bdaddr, addr);
                memset(name, 0, sizeof(name));
                if (hci_read_remote_name(sock, &(ii+i)->bdaddr, sizeof(name), name, 0) < 0)
        		strcpy(name, "[unknown]");
		printf("FOUND: (%d) = %s , %s\n",i , name, addr);		
		
		cb_list = calloc(1, sizeof(b_list));
		cb_list->name = name;
		cb_list->m_addrs = addr;
		cb_list->next = hb_list; /* Warning occurs here line 48 */
		hb_list = cb_list;
		printf("ADDED: (%d) = %s , %s\n", i, hb_list->name, hb_list->m_addrs);
	}	
	
	cb_list = hb_list;

	free( ii );
        close( sock );
	
	printf("RETURNED: %s, %s\n", hb_list->name, hb_list->m_addrs);	
	
	return hb_list;
}

Also, the structure for the linked list b_list is such:

Code:

typedef struct
{
	char* name;
	char* m_addrs;
	struct b_list* next;
} b_list;

Also, if you notice anything else in the code that could be fixed up to run a little better that would be appreciated as well.

Thanks,

Brian

Look at your structure again.

Code:

typedef struct
{
	char* name;
	char* m_addrs;
	struct b_list* next;
} b_list;

Your next pointer is of type struct b_list *, but your hb_list is of type b_list. Which is not acceptable and you are trying to assign of both imcompitable type and hence the warning.

Typecast it to

Code:

cb_list->next = (struct b_list *) hb_list;

Hopefully this should work.

ssharish

> ii = (inquiry_info*)malloc(max_rsp * sizeof(inquiry_info));
Don't cast the result of malloc in a C program, see the FAQ.

> b_list *hb_list, *cb_list;
> hb_list = NULL;
Standard C (the C89 variety) doesn't allow mixed variables and declarations. The new C99 'C' does, as do various compiler extensions.
Consider using "-ansi" as a compile option.

Your linked list should be like this

Code:

typedef struct b_list_tag
{
	char* name;
	char* m_addrs;
	struct b_list_tag* next;
} b_list;

Then you should be able to assign without warnings.

To ssharish2005: Thanks, that worked like a charm.

To Salem: I must have missed that. I have heard it was better to use calloc(max_rsp, sizeof(inquiry_info) instead because it initializes everything to NULL when created.
So I changed it to calloc and it seems to work fine.

When I compile, I get no issues with
b_list *hb_list, *cb_list;
hb_list = NULL;
But, so I understand correctly, would it be better to set it up as two lines?
ie: b_list *hb_list = NULL;
b_list *cb_list;

And lastly, I went back and attempted to make the structure as you have described, but I still get the error. (That is unless I cast as ssharish2005 had said.) I also, don't understand the purpose that b_list_tag has on the program. If you could shed a little more light on this or if you know where I can find an explanationt, it would be appreciated.

Happy Holidays!

Brian

> But, so I understand correctly, would it be better to set it up as two lines?
It would be better to move the declarations to the beginning of the statement block.

> I went back and attempted to make the structure as you have described, but I still get the error.
You'd better post your latest code then, because if done properly, the cast is unnecessary.

> I also, don't understand the purpose that b_list_tag has on the program.

Code:

typedef struct b_list_tag
{
	char* name;
	char* m_addrs;
	struct b_list_tag* next;
} b_list;

// Is equivalent to
struct b_list_tag
{
	char* name;
	char* m_addrs;
	struct b_list_tag* next;
} b_list;
typedef struct b_list_tag b_list;

In your original code, your struct had no name, but you tried to refer to it by the typedef name before the typedef name was known. So what you ended up with was a 'b_list' in two different contexts, and a warning when you tried to assign one to the other.

You can get away with 'typedef struct' having no tag name so long as that struct is not self-referencing. But if it is, as in a linked list, you need the tag name in order to correctly type the next pointer.

> I have heard it was better to use calloc(max_rsp, sizeof(inquiry_info) instead
> because it initializes everything to NULL when created.
Not so. True it initialises everything to 'all-bits-zero', but there are some rare machines which have NULL pointers which are not all bits zero.
http://c-faq.com/malloc/calloc.html
It's better to allocate using malloc, then use type-correct assignments to initialise all the data within the struct you've just created.

Okay, that makes more sense. I have fixed up my code (hopefully) and attempted to continue on. I have encountered another problem which is related when passing a pointer from main.

First off here are the edits to my code so far:

b_poplist.c

Code:

#include "b_list.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <bluetooth/bluetooth.h>
#include <bluetooth/hci.h>
#include <bluetooth/hci_lib.h>
#include <unistd.h>

int b_poplist(b_list *hb_list)
{
	/* Declarations */
	inquiry_info *ii = NULL;
     int max_rsp, num_rsp;
     int dev_id, sock, len, flags;
	int i;
     char addr[19] = { 0 };
     char name[248] = { 0 };
	b_list *nb_list = NULL;
	b_list *cb_list = NULL;
	
	/* Opens Socket */	
     dev_id = hci_get_route(NULL);
     sock = hci_open_dev( dev_id );
     if (dev_id < 0 || sock < 0) {
         perror("opening socket");
         exit(1);
     }

	/* Gets Ready To Scan */
     len  = 8;
     max_rsp = 255;
     flags = IREQ_CACHE_FLUSH;
     ii = malloc(max_rsp*sizeof(*ii));
    
	/* Scans and returns number */
     num_rsp = hci_inquiry(dev_id, len, max_rsp, NULL, &ii, flags);
     if( num_rsp < 0 ) perror("hci_inquiry");

	/* Return 0 if no devices found */
	if(num_rsp == 0)
	{
		free(ii);
		close(sock);
		printf("FOUND: O\n");
		return 0;
	}
	
	/* Creates linked list of devices */	
	for (i = 0; i < num_rsp; i++) {
		ba2str(&(ii+i)->bdaddr, addr);
          memset(name, 0, sizeof(name));
          if (hci_read_remote_name(sock, &(ii+i)->bdaddr, sizeof(name), name, 0) < 0)
        		strcpy(name, "[unknown]");
		printf("FOUND: (%d) = %s , %s\n",i , name, addr);		
		
		/* Creates Next Element */
		nb_list = malloc(sizeof(*nb_list));
		nb_list->name = name;
		nb_list->m_addrs = addr;
		nb_list->next = NULL;
		
		/* Adds element to end of linked list */
		if(hb_list == NULL)
			hb_list = nb_list;
		else
		{
			cb_list = hb_list;		
			while(cb_list->next != NULL)
				cb_list = cb_list->next;
			cb_list->next = nb_list;
		}

		printf("ADDED: (%d) = %s , %s\n", i, hb_list->name, hb_list->m_addrs);
	}	

	/* Free's Up Memory */
	free( ii );
     close( sock );
	
	/* Returns number of entries */
	return i;
}

main.c

Code:

#include "b_list.h"
#include "b_poplist.h"
#include <stdio.h>
#include <stdlib.h>

int main()
{
	int num_entries = 0;
	b_list *newList = malloc(sizeof(*newList));
	newList = NULL;

	num_entries = b_poplist(newList);
	
	if(num_entries > 0)
	{
		printf("%s\n", newList->name);
		printf("%s\n", newList->m_addrs);
	}
	
	free(newList);
	return 0;
}

It compiles just fine, but when run, I get a "Segmentation fault (core dumped)" which I understand occurs when attempting to access memory from somewhere which is not allowed. I have narrowed down where the fault occurs. When I attempt to run printf("%s\n", newList->name); it occurs. I am dumbfound as to why this occurs. In b_poplist.c I have it print out the data in the list brought through after it is manipulated and it is correct. Yet, when I attempt to print it from main I get the error. Thus I am assuming I am only passing by value and not by reference. Any insight would be helpful or where I can find an example of how to correctly pass the pointer into the function.

Thanks for helping me out.
For the last year I've been programming with Java, and before that I was using C++. After reading up on a quick tutorial for C, I thought I'd have a go at it. But these pointer issues are hitting me hard.

Thanks,
Brian

> b_list *newList = malloc(sizeof(*newList));
> newList = NULL;
What do these two lines do, apart from leak memory?

> num_entries = b_poplist(newList);
You're passing the list pointer 'by value', so the function creates the whole list on a copy of the pointer, not your actual list pointer.

Code:

int b_poplist ( b_list **list ) {
    if ( *list == NULL ) {
        *list = malloc( sizeof(b_list) );
    }
}
int main ( ) {
	b_list *newList = NULL;
	num_entries = b_poplist( &newList );
}

So when the function modifies the list, then you're really updating the list pointer in main.

Also, your b_poplist is way too complicated.
Separate into smaller functions which perform a series of specific tasks, like
- opening / closing the connection
- resource inquiry
- reading data
- appending to the list.

Whenever I create a linked list, I start with a few simple primitives to make life easier
- initialise a list
- allocate and initialise a node
- append a node to a list
- dump a list to stdout (for debug)
- delete a list

newLists's memory does not change. What you're doing is passing a pointer to your memory and not your pointer, so newList is not changed inside your b_poplist function.
Instead, you're just creating memory leaks. You need to pass the address of your pointer, not your actual allocate memory. Since the pointer is of type b_list*, you need to take a b_list** as argument. And malloc is unneccesary then, because your b_poplist will allocate and assign memory to the list by itself. And newList is just a local variable.

Hope that makes sense.

Btw, don't mix spaces and tabs. Stick with one and keep your indentation consistent. Don't indent a tab somewhere and 6 spaces somewhere. Use 1 tabs, 2 tabs or 4 spaces, 8 spaces and so on. Choose one.

Thanks a lot for all of your help. I found some documentation that helped me out with pointers so I figured I'd share for anyone else having trouble.

http://cslibrary.stanford.edu/103/

I've considered your recommendations and fixed up my code a bit.

Thanks,
Brian