getting a segfault using pointers

below im trying to create a function makeaddr that will take a few options and take most of the work out of creating a socket, the code compiles ok with no warnings, but it seg faults.

I'm pretty sure it has something to do with the int sock and me passing the socket file descriptor ?

in the socket and bind functions calls i originally didnt have the *s = only had s = but it compiled with warnings and segfaulted.

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h> 
#include <sys/socket.h>
#include <netinet/in.h>
#include <unistd.h>
#include <arpa/inet.h>
#include <netdb.h>
#include <sys/wait.h>
#include <errno.h>


void bail (char *msg){
	fputs(strerror(errno),stderr);
	fputs(": ",stderr);
	fputs(msg,stderr);
	fputc('\n',stderr);
	exit(1);

}


int makeaddr(void * addr, int len, int *s, char *type, char *ip, int port){

	//takes &addr,sizeof(addr), &sock,"TCP",ipaddr, port number
	int Default_port = 3099;
	struct sockaddr_in *ad = (struct sockaddr_in *)addr; 	

	memset(&addr,0,len);

	ad->sin_family = AF_INET;

	if(strcmp(ip,NULL)==0){
	
		ad->sin_addr.s_addr = INADDR_ANY;

	}

	else{
		ad->sin_addr.s_addr = inet_addr(ip);

	}

	if(port == 0){

		ad->sin_port = htons(Default_port);
	

	}
	else{

		ad->sin_port = htons(port);

	}

	
	
	//we have created the address structure. Now create the socket

	if(strcmp(type,"TCP")==0){

		if(*s = socket(AF_INET,SOCK_STREAM,0)== -1)
			bail("TCP Sock Error");
		if(bind(*s,(struct sockaddr*)&addr,len)== -1){

			bail("TCP Bind Error");

		}

		printf("TCP Socket Successfully created: On Port %d \n",(int)ad->sin_port);

	}


	else{


		if(*s = socket(AF_INET,SOCK_DGRAM,0)== -1)
			bail("UDP Sock Error");
		if(bind(*s,(struct sockaddr*)&addr,len)== -1){

			bail("UDP Bind Error");

		}
		printf("UDP Socket Successfully created: On Port %d \n",(int)ad->sin_port);

	}
	
	






}

int main(int argc, char *argv[]){

	int sock;
	struct sockaddr_in addr;

	//takes &addr,sizeof(addr), &sock,"TCP",ipaddr, port number
	makeaddr(&addr,sizeof(addr),&sock,"TCP",NULL,0);

	close(sock);
	return 0;
}

>if(strcmp(ip,NULL)==0)
Yeah, comparing with NULL will do that every time.

ip == NULL is how you test for a NULL pointer (even when they are char*)

Also, what is the point of the first parameter being void* when you just cast back to the same type you started with.

im just trying to get familuar with casting and declaring things as voids i know its stupid but just trying to work out things in my head

Right here:

Code:

	if(strcmp(ip,NULL)==0){
	
		ad->sin_addr.s_addr = INADDR_ANY;

	}

You can't strcmp(NULL,NULL). What you should do here is just:

Code:

if (!ip) ad->sin_addr.s_addr = INADDR_ANY;

! is negation, so that is short for "if (ip == NULL)" (since NULL is false, !ip will be true if ip is false/NULL).

i tried chaning the strcmp to == and im still getting a seg fault.

If addr is already a pointer, why do you keep doing &addr?

Originally Posted by mushy

i tried chaning the strcmp to == and im still getting a seg fault.

Probably a different segfault, since here is how I found the first one (you should learn how to do this, it's easy):

Code:

[root~/C] gcc test.c  // your code
[root~/C] ./a.out
Segmentation fault   // yep
[root~/C] gcc -g test.c
[root~/C] gdb ./a.out
GNU gdb Fedora (6.8-29.fc10)
Copyright (C) 2008 Free Software Foundation, Inc.
(gdb) run
Starting program: /media/sda6/root/C/a.out 

Program received signal SIGSEGV, Segmentation fault.
strcmp () at ../sysdeps/x86_64/strcmp.S:29
29	L(oop):	movb	(%rdi), %al
Current language:  auto; currently asm
(gdb) bt
#0  strcmp () at ../sysdeps/x86_64/strcmp.S:29
#1  0x0000000000400996 in makeaddr (addr=0x0, len=16, s=0x7fffffffdecc, 
    type=0x400c2b "TCP", ip=0x0, port=0) at test.c:34
#2  0x0000000000400b17 in main (argc=1, argv=0x7fffffffdfa8) at test.c:105
(gdb) quit

First, re-compile with the -g switch to add debugging info. Then run gdb on the executable. Type "run" at the prompt after it starts. If you do not see a line from your file, type "bt" for backtrace. Notice the backtrace starts at line 105 (where makeaddr is called in main) and the last place it goes in your program is line 34, where the strcmp() is.

If you don't have gdb installed, do a "apt-cache search gdb" or "yum search gdb" or whatever you usually do to install distro packages (there will be one). gdb will sometime complain "seperate debug-infos not installed", in which case just copy paste that line (it can be a few terminal lines long, beginning with something like "debuginfo-install") onto the command line and do it (you need to be online).

im still not really sure about pointers, i thought

&addr means the address
addr ment the content

before when i used to memset struct sockaddr_in 's i used to use &addr - but i guess this is different because i passed it to a function?

so let me get this right...

if i pass an address to a function

say int name (char *name)

in the function name if i want to change the value of name outside the function i do *name = "john"

if i want to refeer to the address i will just do name

what does &name refer too then?

also in that debug info does that mean the error is coming from the second argument i pass to makeaddr??

After I corrected the strcmp() thing, I didn't get a segfault, I get this:

Socket operation on non-socket: TCP Bind Error
which it thrown by your bail() function.

The first &addr is okay (passing the address of addr from main). The problem is the second one:

Code:

if(bind(*s,(struct sockaddr*)&addr,len)== -1){

addr in makeaddr is a pointer....

Actually I am wrong -- well, at least that is not the problem. This is:

Code:

if(*s = socket(PF_INET,SOCK_STREAM,0)== -1)

What that does is set *s to either 1 (if socket returns other than -1) or 0 (if socket == -1). You need some parantheses:

Code:

if((*s = socket(PF_INET,SOCK_STREAM,0))== -1)

After that, I get:
TCP Socket Successfully created: On Port 0
That's using the 2nd &addr I said was wrong, by the way, which makes me a little suspicious, but anyway.

[edit] oh because of the prior memset:

Code:

 memset(&addr,0,len);

Remove BOTH of the & in makeaddr on addr and you get:
TCP Socket Successfully created: On Port 6924
That's because you need to:

Code:

printf("TCP Socket Successfully created: On Port %d \n",ntohs(ad->sin_port));

ok i now have this

Im confused about the bind command im getting the same error as you, i pass addr but i declare ad

struct sockaddr_in *ad = (struct sockaddr_in *)addr;

and use ad-> to build the address.

When i try and bind that too the socket it doesnt seem to be working?

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h> 
#include <sys/socket.h>
#include <netinet/in.h>
#include <unistd.h>
#include <arpa/inet.h>
#include <netdb.h>
#include <sys/wait.h>
#include <errno.h>


void bail (char *msg){
	fputs(strerror(errno),stderr);
	fputs(": ",stderr);
	fputs(msg,stderr);
	fputc('\n',stderr);
	exit(1);

}

char *resolve (const char *hostname){

	struct hostent *hp;
	hp = gethostbyname(hostname);

	if(!hp){

		bail("Error Resolving host");

	}
	char rstr = sprintf("%s",hp->h_addr_list[0]); 
	return hp->h_addr_list[0];



}


int makeaddr(void * addr, int len, int *s, char *type, char *ip, int port){

	//takes &addr,sizeof(addr), &sock,"TCP",ipaddr, port number
	int Default_port = 3099;
	struct sockaddr_in *ad = (struct sockaddr_in *)addr; 	

	memset(ad,0,len);

	ad->sin_family = AF_INET;

	if(!ip){
	
		ad->sin_addr.s_addr = INADDR_ANY;

	}

	else{
		ad->sin_addr.s_addr = inet_addr(ip);

	}

	if(port == 0){

		ad->sin_port = htons(Default_port);
	

	}
	else{

		ad->sin_port = htons(port);

	}

	
	
	//we have created the address structure. Now create the socket

	if(strcmp(type,"TCP")==0){

		if(*s = socket(AF_INET,SOCK_STREAM,0)== -1)
			bail("TCP Sock Error");
		if(bind(*s,(struct sockaddr*)ad,sizeof(ad))== -1){

			bail("TCP Bind Error");

		}

		printf("TCP Socket Successfully created: On Port %d \n",(int)ad->sin_port);

	}


	else{


		if(*s = socket(AF_INET,SOCK_DGRAM,0)== -1)
			bail("UDP Sock Error");
		if(bind(*s,(struct sockaddr*)ad,len)== -1){

			bail("UDP Bind Error");

		}
		printf("UDP Socket Successfully created: On Port %d \n",(int)ad->sin_port);

	}
	
	






}

int main(int argc, char *argv[]){

	int sock;
	struct sockaddr_in addr;

	//takes &addr,sizeof(addr), &sock,"TCP",ipaddr, port number
	makeaddr(&addr,sizeof(addr),&sock,"TCP",NULL,0);

	close(sock);
	return 0;
}

ok guys thanks for your help finally got it work. appreciate it.

Something im confused about though, i didnt realise i had done it and dont understand why it works.

struct sockaddr_in *ad = (struct sockaddr_in *)addr;

and use ad-> to build the address.

but when i bind i dont use ad i used addr - Does this just mean ad points to addr and when i set the ad vars it changes and sets the addr vars?

Originally Posted by mushy

but when i bind i dont use ad i used addr - Does this just mean ad points to addr and when i set the ad vars it changes and sets the addr vars?

What you are doing there is silly. Yes, they do point to the same place, but you don't need the extra pointer, and your prototype should look like this:

Code:

int makeaddr(struct sockaddr_in *addr, int len, int *s, char *type, char *ip, int port)

Then get rid of ad and just use addr instead (the notation remains the same, just add "dr").

ps. see the 2nd edit at the end of my last post for an explanation of how to get the port number to report correctly.

yea mk i got the port number working it picks a random port which is what i wanted. Thanks for your help. I kinda understand it a bit better now. I feel like im starting to grasp it. Cheers.