Review internet connection pinging method

Question

This is some C code I have to simply test the internet connection. Any comments/tips on efficiency and refactoring this program down would be greatly appreciated.

int testConnection(void)
{
 int status;
 struct addrinfo host_info;
 struct addrinfo *host_info_list;
 memset(&host_info, 0, sizeof host_info);
 #ifdef DEBUG
 fprintf(stdout,"Setting up the structs...");
 #endif
 host_info.ai_family = AF_UNSPEC; // IP version not specified. Can be both.
 host_info.ai_socktype = SOCK_STREAM; // Use SOCK_STREAM for TCP or SOCK_DGRAM for UDP.
 status = getaddrinfo("www.google.com", "80", &host_info, &host_info_list);
 if (status != 0) fprintf(stdout, "Address info error:: %s\n", gai_strerror(status));
 #ifdef DEBUG
 fprintf(stdout, "Creating a socket...\n");
 #endif
 int socketfd ;
 socketfd = socket(host_info_list->ai_family, host_info_list->ai_socktype, host_info_list->ai_protocol);
 if (socketfd == -1) fprintf(stderr, "Socket error\n");
 #ifdef DEBUG
 fprintf(stdout, "Connecting...");
 #endif
 status = connect(socketfd, host_info_list->ai_addr, host_info_list->ai_addrlen);
 if (status < 0) fprintf(stderr, "Error while connecting.\n");
 #ifdef DEBUG
 fprintf(stdout, "Sending message...\n");
 #endif
 const char *msg = "GET / HTTP/1.1\nhost: www.google.com\n\n";
 int len = strlen(msg);
 ssize_t bytes_sent = send(socketfd, msg, len, 0);
 if (bytes_sent == 0) fprintf(stderr, "No bytes sent.\n");
 #ifdef DEBUG
 fprintf(stdout, "Bytes sent: %d\n", bytes_sent);
 fprintf(stdout, "Waiting to recieve data...\n");
 #endif
 char incomming_data_buffer[1000];
 ssize_t bytes_recieved = recv(socketfd, incomming_data_buffer,1000, 0);
 // If no data arrives, the program will just wait here until some data arrives.
 if (bytes_recieved == 0) fprintf(stderr, "Host shut down.\n");
 if (bytes_recieved == -1) fprintf(stderr, "Recieve error.\n");
 incomming_data_buffer[bytes_recieved - 2] = '0円';
 #ifdef DEBUG
 fprintf(stdout, "Bytes recieved: %d\n", bytes_recieved);
 fprintf(stdout, "%s\n", incomming_data_buffer);
 fprintf(stdout, "Receiving complete. Closing socket...\n");
 #endif
 freeaddrinfo(host_info_list);
 close(socketfd);
 #ifdef DEBUG
 fprintf(stdout, "Socket closed.\n");
 #endif
 return 0;
}

Answer 1

Some comments, mostly minor:

I would extract the server connection to a function:

static int connect_server(const struct addrinfo *host_info)
{
 struct addrinfo *host_info_list;
 int fd = -1;
 int status = getaddrinfo(...);
 while(...) {
 fd = socket(...);
 status = connect(...);
 }
 freeaddrinfo(host_info_list);
 return fd;
}

---

All that DEBUG stuff is distracting. Maybe it is temporary, but if you wanted to leave it in, I suggest extracting it:

#include <stdarg.h>
static inline void debug(const char *format, ...)
{
#ifdef DEBUG
 va_list ap;
 va_start(ap, format);
 vfprintf(stdout, format, ap);
 va_end(ap);
#endif
}

and calling it:

debug("Bytes recieved: %ld\n%s\nReceiving complete. Closing socket...\n",
 bytes_recieved,
 incomming_data_buffer);

If DEBUG is undefined, the inline debug function will be empty and will be excluded during compilation - it disappears.

---

You clearly need to loop to read the whole message... After reading you throw away the last two bytes.

buffer[bytes_recieved - 2] = '0円';

The recv call filled the buffer, so to 0円 terminate properly you need to specify a smaller buffer:

char buffer[1000];
ssize_t bytes_recieved = recv(socketfd, buffer, sizeof buffer - 1, 0);
...
if (bytes_recieved > 0) {
 buffer[bytes_recieved] = '0円';
}

Note the use of sizeof instead of an explicit 1000

---

And some other things...

• The man-page for getaddrinfo suggests looping through the list of addresses returned instead of just using the first (in case the first does not work).

• When something fails you should exit rather than continuing.

• instead of using strlen on a constant string, use sizeof:

  const char msg[] = "GET / HTTP/1.1\nhost: www.google.com\n\n";
  ssize_t bytes_sent = send(socketfd, msg, sizeof msg - 1, 0);
  

  note the msg[], not *msg

• perhaps use perror or strerror on failure to read etc.

• camelCase function name but separate_word names elsewhere.