Winsock GET and POST Functions

This works fine so far, but I am looking for some advice as to how to better write this. My goal is to learn and incorporate current/latest C++ practices, and become a better programmer overall.

#include "HTTPRequest.h"

/*
    This function initalized Winsock
*/
bool HTTPRequest::InitWinsock()
{
    WSADATA WsaDat;

    if (WSAStartup(MAKEWORD(2, 2), &WsaDat) != 0)
    {
        std::cout ai_addr)->sin_addr));
    servAddr.sin_port               = htons(Port);

    theSocket = socket(AF_INET, SOCK_STREAM, 0);
    if (theSocket == INVALID_SOCKET)
    {
        std::cout > vPostReqs)
{
    int nret;
    WSADATA WsaDat;
    SOCKET theSocket;

    if (WSAStartup(MAKEWORD(2, 2), &WsaDat) != 0)
    {
        std::cout ai_addr)->sin_addr));
    servAddr.sin_port = htons(Port);

    theSocket = socket(AF_INET, SOCK_STREAM, 0);
    if (theSocket == INVALID_SOCKET)
    {
        std::cout > vPostReqs;

    vPostReqs.push_back(std::make_pair("ajax_login", "yes"));
    vPostReqs.push_back(std::make_pair("ACT", "10"));
    vPostReqs.push_back(std::make_pair("RET", "-2"));
    vPostReqs.push_back(std::make_pair("site_id", "1"));
    vPostReqs.push_back(std::make_pair("username", "Boobin"));
    vPostReqs.push_back(std::make_pair("password", "inboob"));
    vPostReqs.push_back(std::make_pair("auto_login", "1"));
    vPostReqs.push_back(std::make_pair("submit", "Submit"));

    hReq.postWebPage("postcatcher.in", 80, "/catchers/55f09e1c23e9dc0300001628", vPostReqs);

    system("pause");
    return(0);
}

Don't use system(). It's generally considered bad practice and pause does not have a place in most programs. If you do intend on using system(), keep in mind that you have to explicitly flush std::cout before the call.

Even then, it's likely that you want an explicit flush for your error messages. Either use std::cerr or std::cout.flush() after each message.

You have InitWinsock(), you never use it

WsaData is a local variable in InitWinsock() making it completely useless.

You also never use WsaData. However after reading the documentation, I'm not convinced you need to call InitWinsock() more than once. So just remove every instance of WsaStartup(). Here's example code from MSDN:

WORD wVersionRequested;
WSADATA wsaData;
int err;

wVersionRequested = MAKEWORD( 2, 2 );

err = WSAStartup( wVersionRequested, &wsaData );
if ( err != 0 ) {
    /* Tell the user that we could not find a usable */
    /* WinSock DLL.                                  */
    return;
}

/* Confirm that the WinSock DLL supports 2.2.*/
/* Note that if the DLL supports versions greater    */
/* than 2.2 in addition to 2.2, it will still return */
/* 2.2 in wVersion since that is the version we      */
/* requested.                                        */

if ( LOBYTE( wsaData.wVersion ) != 2 ||
        HIBYTE( wsaData.wVersion ) != 2 ) {
    /* Tell the user that we could not find a usable */
    /* WinSock DLL.                                  */
    WSACleanup( );
    return; 
}

/* The WinSock DLL is acceptable. Proceed. */

Error messages

The recommended way to get an error is to use WSAGetLastError(), not errno:


Error codes set by Windows Sockets are not made available through the
errno variable. Additionally, for the getXbyY class of functions,
error codes are not made available through the h_errno variable. The
WSAGetLastError function is intended to provide a reliable way for a
thread in a multithreaded process to obtain per-thread error
information.

Furthermore, the error constants prefixed by WSA are preferred:

// Example from MSDN
r = recv(...);
if (r == -1       /* (but see below) */
    && WSAGetLastError() == WSAEWOULDBLOCK)
    {...}

Furthermore, the error messages should be more descriptive. You may either choose to convert the error code to a string yourself or use FormatMessage as MSDN recommends.

Minor issue

memset(&servAddr, 0, sizeof(servAddr)); is not necessary in C++. You can zero initialize a struct with servAddr = {0};.

struct some_struct is redundant in C++. You can omit the struct keyword.

(LPCSTR)some_string.c_str() is redundant. LPCSTR is just a typedef to const char*.

StackExchange Code Review Q#104262, answer score: 2