Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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?

share|improve this question
1  
You might be testing whether you're in a country that blocks Google from time to time for political reasons. –  200_success Jun 11 at 4:19
add comment

2 Answers 2

up vote 4 down vote accepted

Testing a URL to see if it is accessible is not a great way to test for an internet connection. There are a number of things I can see that can go wrong (or things you may be testing other than 'the internet'):

  • you are testing your DNS server (if it is down, you have 'no internet', even though you do....)
  • you are testing that caching-web-proxy on your company firewall that has a cache of the google page.....
  • you are establishing a socket connection (slow in comparison) when there is no need for that
  • on that socket you are establishing a protocol communication (HTTPS which is unnecessary)
  • you are testing against one hard-coded destination that could change (I would guess that http will be discontinued on google 'soon' and they will swap to https.... - oh, you are using HTTPS already, which requires multiple packets!)
  • you are testing against just one site, which may be a victim of a denial-of-service, or downtime.

The right tool for the job is an ICMP packet (or few) ... a 'ping'.

An ICMP packet is designed to be a lightweight system for identifying hosts that are up.

By using a known IP address, you do not need to worry about DNS issues By using multiple IP's you are covered better for system downtime. By using a configurable system to change the IPs if needed.

Consider the IP's 8.8.8.8 <-- google's DNS server

and, for more, I found this link... (I used 8.8.8.8 from before).

share|improve this answer
    
As long as the library check the certificate, you don't need to worry about a proxy having cached the result. Whether https is overkill and echo request is sufficient depends on the requirements. Using somebody else's site isn't great practice either way, better use your own site. –  kasperd Jun 11 at 7:18
add comment

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.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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