Thread: Bug hunting with Valgrind

I'm using Valgrind to find if there's bug in my program (i'm coding under linux) and it find someting, but i don't understand where's the problem.

Valgrind sais:

==5397== Thread 2:
==5397== Conditional jump or move depends on uninitialised value(s)
==5397== at 0x401358: ListSearch (packethandler.c:97)
==5397== by 0x40177D: HandlePackets (packetreader.c:47)
==5397== by 0x401643: Listen (packethandler.c:146)
==5397== by 0x4E2BFC6: start_thread (in /lib/libpthread-2.7.so)
==5397== by 0x51105AC: clone (in /lib/libc-2.7.so)
==5397==
==5397== Jump to the invalid address stated on the next line
==5397== at 0x0: ???
==5397== by 0x401643: Listen (packethandler.c:146)
==5397== by 0x4E2BFC6: start_thread (in /lib/libpthread-2.7.so)
==5397== by 0x51105AC: clone (in /lib/libc-2.7.so)
==5397== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==5397==
==5397== Process terminating with default action of signal 11 (SIGSEGV)
==5397== Bad permissions for mapped region at address 0x0
==5397== at 0x0: ???
==5397== by 0x401643: Listen (packethandler.c:146)
==5397== by 0x4E2BFC6: start_thread (in /lib/libpthread-2.7.so)
==5397== by 0x51105AC: clone (in /lib/libc-2.7.so)
--5397-- REDIR: 0x50b69e0 (free) redirected to 0x4c20b00 (free)

For the first error valgrind is referring to that conditional jump:

Code:

RecvPacketList *ListSearch(RecvPacketList *plist, unsigned char i)
{
        while (plist != NULL)<- THIS ONE
        {
                if (plist->ID == i)
                {
                        return plist;
                }
                plist = plist->next;
        }
        return NULL;
}

ListSearch is called here:

Code:

void HandlePackets(RecvPacketList **plist, char *recvBuff)
{	
	unsigned int ID = -1;
	RecvPacketList *list;
	while(ID != 0)
	{
		ID = (unsigned int)*recvBuff;
		recvBuff++;
		list = ListSearch(*plist, ID);
		if(list != NULL){
			pFunction pfunc = list->func;
			(*pfunc)(&recvBuff);
		}
	}
}

And going again "back", HandlePackets is called here:

Code:

void *Listen(void *args)
{
	char recvPacketBuff[1024] = {0};
	RecvPacketList *plist;
	fd_set fset;
	FD_ZERO(&fset);
	printf("Initializing Packet List: ");
	Initialize(&plist);
	printf("Done\n");
	
	while(1)
	{
		FD_SET(sock_fd, &fset);
		select(sock_fd + 1, &fset, NULL, NULL, NULL);
		if(FD_ISSET(sock_fd, &fset)){
			recv(sock_fd, &recvPacketBuff, sizeof(recvPacketBuff), 0);
			HandlePackets(&plist, recvPacketBuff); <- THE SECOND ERROR, ln 146
			CleanBuff(recvPacketBuff);
		}
	}
}

And as you can see here.. plist IS initialized:

Code:

void Initialize(RecvPacketList **plist)
{
	pFunction temp = NULL;
	*plist = malloc(sizeof(RecvPacketList));
	if(*plist == NULL){
		printf("Error to allocate space for plist");
		exit(1);
	}
	(*plist)->ID = 0;
	(*plist)->func = NULL;
	Register(0x0B,plist, temp = DamagePacket);	
	Register(0x24,plist, temp = DisplayBuyList);
	Register(0x25,plist, temp = TradeEquipOp);
	Register(0x3C,plist, temp = VendorBuyContentOp);
	Register(0x6F,plist, temp = SecureTradeOp);
	Register(0x74,plist, temp = VendorBuyList);
	Register(0x82,plist, temp = AccountLoginRej);
	Register(0xA8,plist, temp = AccountLoginAck);
	Register(0xBA,plist, temp = ArrowOp);
	Register(0xB7,plist, temp = ObjectHelpResponse);
}

If you want also to see register:

Code:

void Register(unsigned char value, RecvPacketList **plist,  pFunction func)
{
	RecvPacketList *nextlist = (RecvPacketList *)malloc(sizeof(RecvPacketList));

	if(nextlist == NULL){
		printf("Memory allocation failed for nextlist");
		return;
	}
 	nextlist->next = *plist;
	*plist = nextlist;
	nextlist->ID = value;
	nextlist->func = func;

	
}

Anyway plist is a linked list that contains "static" packet informations (packet to be read).
I also highlighted where's the second error.

Originally Posted by Salem

> In the function it allocates memory for that pointer, it set ID to 0 and func to NULL
But it doesn't set ->next to NULL (which is what you eventually get to test, and valgrind objects to)

Oh lol it was so simple XD, but i thought valgrind was referring to the last object i added to the list :\

Originally Posted by Salem

> then Register function appends other RecvPacketList structures
Being pedantic, it prepends to the front of the list, not appends to the end.

Don't worry you're not pedantic :P and i'm not english, i chose the wrong word ^^

Originally Posted by Salem

> Register(0x0B,plist, temp = DamagePacket);
Do all these temp = do anything useful?

No, it was an old try, when i wasn't sure if had to pass the function by name or pass it as a pointer (so using a variable).
I removed it.
Aniway there's still the second problem...

Yes i'm referring to the jump, and i still have that problem, i only changed a line to solve the first problem.

This:

Code:

void Initialize(RecvPacketList **plist)
{
	pFunction temp = NULL;
	*plist = malloc(sizeof(RecvPacketList));
	if(*plist == NULL){
		printf("Error to allocate space for plist");
		exit(1);
	}
	(*plist)->ID = 0;
	(*plist)->func = NULL;
	Register(0x0B,plist, temp = DamagePacket);	
	Register(0x24,plist, temp = DisplayBuyList);
	Register(0x25,plist, temp = TradeEquipOp);
	Register(0x3C,plist, temp = VendorBuyContentOp);
	Register(0x6F,plist, temp = SecureTradeOp);
	Register(0x74,plist, temp = VendorBuyList);
	Register(0x82,plist, temp = AccountLoginRej);
	Register(0xA8,plist, temp = AccountLoginAck);
	Register(0xBA,plist, temp = ArrowOp);
	Register(0xB7,plist, temp = ObjectHelpResponse);
        etc....................................
}

Now is (i also removed the useless temp variable ^^):

Code:

void Initialize(RecvPacketList **plist)
{
	*plist = malloc(sizeof(RecvPacketList));
	if(*plist == NULL){
		printf("Error to allocate space for plist");
		exit(1);
	}
	(*plist)->ID = 0;
	(*plist)->func = NULL;
	(*plist)->next = NULL; <- THIS IS THE NEW LINE
	Register(0x0B,plist, DamagePacket);	
	Register(0x1A,plist, WorldItem);
	Register(0x1D,plist, RemoveItem);
        etc............................
}

The fact is that pfunc cannot be null if ListSearch find a valid ID, it was my fault to initialize plist->ID (in Initialize function) to 0, because when HandlePacket encounter a 0 ID in the buffer and use ListSearch, this function returns the structure with all NULL, so to resolve i simply initialized ID to -1.
Thnx to all, stupid bugs, the next time is better to think more eheh!