2

Let's send a message to C-SERVER:

echo '1234567890' | nc <ip> <port>

C-SERVER prints: ( oo.ooo.o = partial IP of client )

678oo.ooo.o---

But I only asked it to print 3 characters from the incoming message.
( starting from 5th character )

strncpy(ii2, iii + 5, 3);

it should only be printing: ( plus the --- )

 678

Question: WHY is it attaching a partial IP address of the client to the string above ?

EXTRA INFO:

The problem seems to go away when I remove this line all together:

snprintf(iip, 15, "%s", inet_ntoa(cli_addr.sin_addr));

But I do not want to remove that line.

FULL CODE:

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

char * nnn;
char iii [500];
char ii2 [3];
char iip [15];


int main()
{
  int one = 1, client_fd;
  struct sockaddr_in svr_addr, cli_addr;
  socklen_t sin_len = sizeof(cli_addr);

  int sock = socket(AF_INET, SOCK_STREAM, 0);
  if (sock < 0)
    err(1, "can't open socket");

  setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(int));

  int port = 86;
  svr_addr.sin_family = AF_INET;
  svr_addr.sin_addr.s_addr = INADDR_ANY;
  svr_addr.sin_port = htons(port);

  if (bind(sock, (struct sockaddr *) &svr_addr, sizeof(svr_addr)) == -1) {
    close(sock);
    err(1, "Can't bind");
  }

  listen(sock, 5);
  while (1) {
    client_fd = accept(sock, (struct sockaddr *) &cli_addr, &sin_len);
    snprintf(iip, 15, "%s", inet_ntoa(cli_addr.sin_addr));
    ssize_t t = read(client_fd,iii,500);

    if ( t > 0 ) { iii[ t ] = '\0'; ii2[ t ] = '\0'; }

    strncpy(ii2, iii + 5, 3);

    printf(ii2);
    printf("---");
    printf("\n");
    asprintf(&nnn, "%s\n", ii2);


    write(client_fd, nnn, strlen(nnn));
    close(client_fd);
  }
}
  • 1
    You forget the string terminator needed when you copy to `ii2`. – Some programmer dude Feb 19 '17 at 13:29
  • @Some programmer dude, can you tell me how it can be done ? I am not good with C (yet). –  Feb 19 '17 at 13:30
  • First of all, for a string of three characters you need an array of *four* characters because of the terminator. Secondly, `strncpy` might not add a terminator so you have to do it explicitly (like e.g. `ii2[3] = '\0'`). Unrelated to your problem, stop using global variables, and please try to come up with some *descriptive* variable names. Learning good habits early will help you later. – Some programmer dude Feb 19 '17 at 13:34
  • I'm not sure what the point of `ii2` is, but it almost certainly should be a local variable. However, if the only reason is to print 3 characters starting with the sixth character of `iii`, the variable and the copy are unnecessary: `printf("%.3s", iii+5);` will do just that. – rici Feb 19 '17 at 16:36

3 Answers3

4

You are using strncpy incorrectly. This function is designed for use with fixed-length strings - something that is rarely used these days.

The function copies three characters into ii2, but it does not add a null terminator. Replace the call with a call to memcpy, and set '\0' in the last character, like this:

char ii2 [4]; // <<== 3 plus 1 for '\0'
...
memcpy(ii2, iii + 5, 3);
ii2[3] = '\0';

Note that if all you need is to print three characters from a possibly longer string, you do not need ii2. You can use %.3s format specifier, like this:

asprintf(&nnn, "%.3s\n", iii+5);

You need to call free(nnn) to avoid creating a memory leak. In addition, since the size of the result is limited to 5, you can replace asprintf call with sprintf to a buffer of fixed size.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
2

Adding to dasblinkenlight's answer.

char ii2 [3];

ssize_t t = read(client_fd,iii,500);

if ( t > 0 ) { iii[ t ] = '\0'; ii2[ t ] = '\0'; }

This looks like a recipe for SegFault. Trying to store value at a location which is well beyond the ii2 array size. (Worst case: t = 500)

rootkea
  • 1,474
  • 2
  • 12
  • 32
1

Try to increase char iip [15]; size to 16. Its common issue for beginers to forgot preserve room for '\0' character. char ii2 [3]; is also too short to store string with the length of 3, so I suggest to increase the room to 4.

Mykola
  • 3,343
  • 6
  • 23
  • 39