DownloadFile Function (using LibCurl)

Question

Please tell me if this function is correct (I believe it to be), efficient, and clean.

Function to check:

::CURLcode CURL_DownloadFile( __in ::LPCSTR URL, __in ::LPCSTR Destination )
{
    ::CURL* Curl;
    ::CURLcode Return_CurlCode;
    std::FILE* File;

    if( ( Curl = ::curl_easy_init( ) ) )
    {
        File = std::fopen( Destination, "wb" );

        ::curl_easy_setopt( Curl, CURLOPT_URL, URL );
        ::curl_easy_setopt( Curl, CURLOPT_WRITEFUNCTION, WriteData );
        ::curl_easy_setopt( Curl, CURLOPT_WRITEDATA, File );

        Return_CurlCode = ::curl_easy_perform( Curl );
        std::fclose( File );
    } else { ( Return_CurlCode = CURLE_FAILED_INIT ); }

    return( Return_CurlCode );
};

Example of usage:

#include "Global.h"

::CURLcode CURL_DownloadFile( __in ::LPCSTR URL, __in ::LPCSTR Destination );

int main( void )
{
    if( CURL_DownloadFile( "http://stackoverflow.com/", "c:\\users\\xorr\\desktop\\stackoverflow.txt" ) != CURLE_OK )
        std::printf( "An error has occured...\n" );

    std::cin.get( );
    return( EXIT_SUCCESS );
    // [email protected]
};

size_t WriteData( void *ptr, size_t size, size_t nmemb, std::FILE *stream ) {
    size_t written;
    written = fwrite(ptr, size, nmemb, stream);
    return written;
}

::CURLcode CURL_DownloadFile( __in ::LPCSTR URL, __in ::LPCSTR Destination )
{
    ::CURL* Curl;
    ::CURLcode Return_CurlCode;
    std::FILE* File;

    if( ( Curl = ::curl_easy_init( ) ) )
    {
        File = std::fopen( Destination, "wb" );

        ::curl_easy_setopt( Curl, CURLOPT_URL, URL );
        ::curl_easy_setopt( Curl, CURLOPT_WRITEFUNCTION, WriteData );
        ::curl_easy_setopt( Curl, CURLOPT_WRITEDATA, File );

        Return_CurlCode = ::curl_easy_perform( Curl );
        std::fclose( File );
    } else { ( Return_CurlCode = CURLE_FAILED_INIT ); }

    return( Return_CurlCode );
};

Answer 1

Your write function is incorrect:

size_t WriteData( void *ptr, size_t size, size_t nmemb, std::FILE *stream ) {
    size_t written;
    written = fwrite(ptr, size, nmemb, stream);
    return written;
}

CURL will abort the transfer if you do not return size*nmemb. So you need to take care to make sure that you actually write all the data provided.

Also this is the action of the default CURLOPT_WRITEFUNCTION so there is no need to write your own version.

You don't check if the File pointer is NULL

    File = std::fopen( Destination, "wb" );

This is a C function (even if you have used it from C++). This means you must check the return value to make sure the faction succeeded.

Don't artifically stop the application. You may forget to remove this and then it will make the program look like it is broken.

std::cin.get( );

If you run from the command line it is not needed. If you are running inside an IDE set up your IDE so that it does not close the output window when the application finishes.

If your program always succeeded. Then I prefer not to return an exit value from main(). The compiler in this one special case will plant a return 0; for you to indicate success. This is a good indication that you have designed your application so that it never fails.

return( EXIT_SUCCESS );

I only return a value if there is a potential for failure and I want to be able to indicate the possible failure states.