View Full Version : C and Pointers = ARRGH!
CraigWatson
March 23rd, 2010, 08:00 PM
I'm having a major headache with some code, and it's down to my (rather crap) understanding of pointers.
I have the following code fragment:
------- Header file -------
#define SOME_PORT 60666
------- main.c -------
create_socket_thread(SOME_PORT);
------- in sockets.c -------
/** Creates a thread for the socket connection **/
void create_socket_thread(int port){
pthread_t thread;
int *ptr = &port;
printf("Will be listening on port %d\n",*ptr);
pthread_create(&thread, NULL, create_socket, (void *) ptr);
}
/** Function adapted from Dave Price's "Network Programming From C" slides **/
void *create_socket(void *port_number){
int port = *((int *) port_number);
printf("Listening on port %d\n",port);
}
The rest of the code is unimportant and works, however all I want the function to do at the moment is output an integer value of the port number. Current output is:
Will be listening on port 60666
Listening on port 0
Is there anyone who can actually fix the code above and explain the principles behind where I'm going wrong? :)
MadCow108
March 23rd, 2010, 08:21 PM
the argument port of create_socket_thread gets destroyed when the function ends.
This means when the thread starts, the pointer is very likely to be invalid.
you have to copy the value into the thread by saving the value in the pointer variable :
pthread_create(&thread, NULL, create_socket, (void *) port);
in the thread:
int port = (int)port_number; // an extra long cast may be required for portability
or make sure the value the pointer points to stays valid for the entire required lifetime (e.g. by letting the creater thread wait for the thread to end with pthread_join)
MindSz
March 23rd, 2010, 08:21 PM
My pointer programming is very rusty, but there's a couple of things you can try. First, shouldn't integers have a value between -32,767 and 32,767?
Also, instead of doing everything in one line like:
int port = *((int *) port_number);
you can try to do it step by step and check the values you are getting at each step (either with gdb or with print statements).
anyway, good luck!
EDIT: OR, you can try to do what MadCow said :)
n0dix
March 23rd, 2010, 08:24 PM
Try with:
int port = (int) port_number;
instead
int port = *((int *) port_number);
CraigWatson
March 23rd, 2010, 08:29 PM
the argument port of create_socket_thread gets destroyed when the function ends.
This means when the thread starts, the pointer is very likely to be invalid.
you have to copy the value into the thread by saving the value in the pointer variable :
pthread_create(&thread, NULL, create_socket, (void *) port);
in the thread:
int port = (int)port_number; // an extra long cast may be required for portability
or make sure the value the pointer points to stays valid for the entire required lifetime (e.g. by letting the creater thread wait for the thread to end with pthread_join)
MadCow: that worked, thanks! Just a couple of minor niggles though, I'm trying to work out as many compilation warning as possible, and I get these for the void and int casts:
cast to pointer from integer of different size
MadCow108
March 23rd, 2010, 08:34 PM
you probably have a 64 bit machine?
this means pointers are 8 byte whereas ints are 4bytes
so to cease the warning save the port in a type with the same size as the pointer.
intptr_t defined in inttypes.h is guaranteed to have the right size:
#include <inttypes.h>
intptr_t port = 1024;
...
intptr_t port = (intptr_t)port_number
to print this number portably you will need the macros also defined in inttypes.h
http://www.opengroup.org/onlinepubs/000095399/basedefs/inttypes.h.html
if you don't care just use %ld
CraigWatson
March 23rd, 2010, 08:40 PM
you probably have a 64 bit machine?
this means pointers are 8 byte whereas ints are 4bytes
so to cease the warning save the port in a type with the same size as the pointer.
intptr_t defined in inttypes.h is guaranteed to have the right size:
#include <inttypes.h>
intptr_t port = 1024;
...
intptr_t port = (intptr_t)port_number
Thanks :) After a bit of Googling I can see that I'm probably going to end up creating more problems by using intptr_t instead of int, so I'll probably end up chasing my own tail so-to-speak for the sake of a couple of compiler warnings.
Are there any adverse effects of using int vs intptr_t? Thanks for the help :)
MadCow108
March 23rd, 2010, 08:47 PM
I think on x86 machines your fine.
But on other architectures there may be problems.
What problems do you see in using intptr_t?
the only drawback I see is the increased typing needs for printing.
But for debugging you can just stick to %ld to save some letters.
An addition to the second possibility mentioned above to avoid all this:
you could dynamically allocate the port number in the thread creator and let the thread take care of the deallocation.
dwhitney67
March 24th, 2010, 12:19 PM
Port numbers range from 0 to 65,535 (2^16 - 1), so technically you should consider declaring the port number to be an unsigned short.
As for declaring your port number using a macro, this should be avoided for two reasons: 1) no type is associated with a macro declaration, and 2) the macro's value is not stored within the program space of the application -- thus it is not addressable.
Consider the following:
#include <pthread.h>
#include <stdio.h>
void* create_socket(void* arg)
{
unsigned short port = *(unsigned short*)(arg);
printf("thread is using port = %u\n", port);
return 0;
}
pthread_t create_socket_thread(const unsigned short* port)
{
pthread_t tid;
pthread_create(&tid, 0, create_socket, (void*) port);
return tid;
}
int main(void)
{
const unsigned short port = 25000;
pthread_t tid = create_socket_thread(&port);
pthread_join(tid, 0);
return 0;
}
P.S. Feel free to replace the 'unsigned short' with 'int'; the main point above is to avoid using a macro.
MadCow108
March 24th, 2010, 02:03 PM
which macro are you talking about?
intptr_t is a typedef
the macros I mentioned are just needed for portable use in the format part of printf/scanf like statements
edit: this is incorrect:
using short in dwhitney's way will still trigger the differing size warning on most systems
editend
here pseudo code of what I think is the best, most portable and warning free way (next to letting the creator wait for the thread to end):
create_thread()
{
// use whatever type you wish
unsigned short * port_ptr = (unsigned short *)malloc(sizeof(unsigned short));
*port_ptr = 1024;
pthread_create(..., (void*)port_ptr);
}
thread(..., void *arg)
{
unsigned short port = *(unsigned short*)arg;
free(arg);
}
dwhitney67
March 24th, 2010, 02:18 PM
which macro are you talking about?
From the OP's opening post:
------- Header file -------
#define SOME_PORT 60666
using short in dwhitney's way will still trigger the differing size warning on most systems
Which systems? I tested the code on Ubuntu 9.10 (32-bit), Mac OS-X (32 bit), and RHEL 5.8 (64-bit). No warnings, no errors!
MadCow108
March 24th, 2010, 02:34 PM
I'm sorry I misinterpreted your code wrong. It is indeed warning free.
johnl
March 24th, 2010, 06:14 PM
Just to chime in, you can avoid the compiler warning and have your code be 'correct' by using a somewhat ugly double cast:
/* port is of type int */
pthread_create(&tid, 0, create_socket, (void*)(intptr_t)port);
Casting port to intptr_t will widen it to 64-bits (on a machine with 64-bit pointers), and then casting to void* from inptr_t is OK.
Powered by vBulletin® Version 4.2.2 Copyright © 2024 vBulletin Solutions, Inc. All rights reserved.