HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Pinging a URL with libcurl

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withlibcurlurlpinging

Problem

I've asked a question about pinging a URL before, but I wanted something that was a bit shorter and still just as efficient. libcurl seemed to be the perfect answer. Here is my method:

/**
 * @fn int testConnection(void)
 * @brief Pings "https://www.google.com/" to test if there is an internet connection.  Timeout is set to three seconds.
 * @return Success value if connected to the internet.
 */
int testConnection(void)
{
    CURL *curl;
    CURLcode res = 0;
    curl_global_init(CURL_GLOBAL_ALL);
    curl = curl_easy_init();
    if(curl)
    {
        curl_easy_setopt(curl, CURLOPT_URL, "https://www.google.com/");
        curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 3);
        curl_easy_setopt(curl, CURLOPT_NOBODY, 1);
        res = curl_easy_perform(curl);
        curl_easy_cleanup(curl);
    }
    curl_global_cleanup();
    return res;
}


Is there anything "wrong" with this pinging method? Are there any ways that I could make this method more efficient?

Solution

Unless you're using C89, I would define variables where they are used (I'm looking at curl).

0 is currently interpreted as CURLE_OK. This means that if cURL fails to initialize, you're returning a success code. That's probably not what you meant to do.

This is a great example of why you should never use plain constants to set the value of an enum. You throw away the entire purpose of the enum when you do that, and lost meaning gives you cases like this.

I would try to hide the cURL dependency. What if you later decide to change to using sockets again? The simplest way to do this would be a simple enum with 3 possibilities: CONNECTION_SUCCESS, CONNECTION_FAILURE, CONNECTION_EXCEPTION. The first two are obvious. The third would be if something other than simple pass/fail happened (cURL can't initialized for example).

This does unfortunately mean you need to do a bit more boiler plate for your error handling, but it means that you can change out the underlying implementation as you please.

Your comment is very vague with regards to return value ("Success value if connected to the internet."). What in the world is a 'success value'? Also, it might be nice to clarify what connecting to the internet means (in this context, it means DNS resolution, connecting and making an HTTP request -- that's a lot more than simple connection).

(Had to pick on you about documentation, of course :p).

I have a lot of reservations about this in general. What if google is down (yeah, yeah, but it can happen!). What if 5 years from now google doesn't exist? What if the user's DNS server enforces Bing (crazy, but you never know). It just seems a really fragile dependency. Unfortunately though, I have no better solution off the top of my head other than check more than 1 site and allow the user to override defaults.

Context

StackExchange Code Review Q#53938, answer score: 8

Revisions (0)

No revisions yet.