array value changing unexpectedly after recv()

Question

after i run the server, i run two instances of client. Now first time i run the client, the received data is saved into array[0], but when i run the second client, the value of array[0] is overwritten with the new value surprisingly. The new data shouls be saved in array[1] instead of overwriting. what mistake am i making?

char *Array[100]; int ArrayCount = 0; are global variables.

void *server()
{
int listenfd = 0;
connfd = 0;
struct sockaddr_in serv_addr;

listenfd = socket(AF_INET, SOCK_STREAM, 0);
memset(&serv_addr, '0', sizeof(serv_addr));

serv_addr.sin_family = AF_INET;
serv_addr.sin_addr.s_addr = htonl(INADDR_ANY);
serv_addr.sin_port = htons(atoi(port));

if(bind(listenfd, (struct sockaddr*)&serv_addr, sizeof(serv_addr))<0)
{
    perror("bind");
    exit(1);
}

listen(listenfd, 10);

while(1)
{
    connfd = accept(listenfd, (struct sockaddr*)NULL, NULL);

    printf("data in Array[0] before recv : %s\n",Array[0]);

    int ns;
    char revdData[2000];
    bzero(revdData,2000);
    ns = recv(connfd,revdData,2000,0);
    close(connfd);

    printf("data in Array[0] after recv : %s\n",Array[0]);

    Array[ArrayCount] = revdData;
    ArrayCount = ArrayCount+1;
 }
}

client code

void *client()
{

int soctype = SOCK_STREAM;
struct hostent *hp, *gethostbyname();
struct sockaddr_in servR;
struct servent *seR;
int tempSocket;
char *host = "localhost";

if ((tempSocket = socket(AF_INET, soctype, 0)) < 0)
{
    perror("socket");
    exit(1);
}

if ((hp = gethostbyname(host)) == NULL)
{
    exit(1);
}

servR.sin_family = AF_INET;
memcpy(&servR.sin_addr, hp->h_addr, hp->h_length);
if (isdigit(*port))
{
    servR.sin_port = htons(atoi(port));
}
else
{
    if ((seR = getservbyname(port, (char *)NULL)) < (struct servent *) 0)
    {
        perror(port);
        exit(1);
    }
    servR.sin_port = seR->s_port;
}


if (connect(tempSocket, (struct sockaddr *) &servR, sizeof(servR)) < 0)
{
    perror("connect");
    exit(1);
}

char input[100];
fgets(input,100,stdin);
int n;
n=send(tempSocket,input,100,0);
if (n < 0)
{
    error("Send");
}
}

output is as follows:

data in Array[0] before recv : (null)
data in Array[0] after recv : (null)
data in Array[0] before recv : 1st Instance
data in Array[0] after recv : 2nd Instance

ideally data after recv should be "1st instance", because i am printing array[0].

Answer 1

This assignment in the server code is not doing what you want:

Array[ArrayCount] = revdData;

This is invalid code, because revdData is a local array with scope delimited to server(). The assignment doesn't copy the array, it copies the pointer to its first element. So, by the time you return from server(), Array contains a pointer to an invalid location - the memory for the local revdData array is invalid. What happens next is unknown. Usually, since you'll be calling server() again, it is very likely that the same memory space is being reused for the new revdData array, so it ends up overwriting what you don't want.

To fix it, you must either allocate memory in Array[ArrayCount] and then use strncpy, or use strndup.

Here's what you can do with strncpy:

Array[ArrayCount] = malloc(ns);
strncpy(Array[ArrayCount], revdData, (size_t) ns);

Also, you can change ns from int to ssize_t, which is the return type of recv.

If you want to use strndup, you don't need to explicitly use malloc, however, if your data contains null bytes, it will not copy beyond that point, and if it doesn't, it will add a null byte in the duplicated string, which might not be what you want.

Note that this approach introduces the problem of memory management; at some point, you have to free the memory allocated for each position taken in Array. You are responsible for doing this, although in small programs you can usually rely on the fact that all allocated memory is freed by the operating system upon program termination.

Answer 2

You are just copying the pointer revdData into Array[i]. hence, all your Array elements just point to revdData.

You should allocate some memory to your Array elements and copy the value of revdData into Array[ArrayCount].

// Just after you recv
Array[ArrayCount] = malloc(sizeof(char) * ns);
strncpy(Array[ArrayCount], revdData, (size_t)ns);
ArrayCount++;


// Free allocated memory before you exit
for(i = 0; i < ArrayCount; i++) {
    free(Array[ArrayCount]);
    Array[ArrayCount] = 0;
}

Please refer to some tutorials for better client/server designs.