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...");
    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");
    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...");
    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");
    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");
    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");
    #ifdef DEBUG
    fprintf(stdout, "Socket closed.\n");
    return 0;

1 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(...);
    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);

and calling it:

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

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.


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.