0

I'm trying to setup a simple TCP client. I don't want the full setup and connect code in my main() so I'm trying to move it to a function but keep getting a Access Violation Error in VS2015, all includes are correct. The function is a straight copy from MSDN.

argc = 2, argv[1] = "localhost", PORT = 27015

Calling function:

win32_connect(&argc, &argv, PORT);

Receiving function:

win32_connect(int *argc, char** argv[], int port)
{
    ...
    iResult = getaddrinfo(argv[1], port, &hints, &result); <- Access Violation
    ...
}

I just can't figure out a proper way to pass the arguments to the function properly so it works as it does when I have the whole function in main()

Header file (cHeader):

#define _CRT_SECURE_NO_WARNINGS
#ifndef CHEADER_H
#define CHEADER_H

//- Includes
#include <sys/types.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <errno.h>
#include <fcntl.h>

//- OS specific includes
#ifdef _WIN32
    #define WIN32_LEAN_AND_MEAN
    #include <windows.h>
    #include <winsock2.h>
    #include <ws2tcpip.h>
    #pragma comment (lib, "Ws2_32.lib")
    #pragma comment (lib, "Mswsock.lib")
    #pragma comment (lib, "AdvApi32.lib")
#endif
....

main():

int main(int argc, char *argv[])
{
    //- Define variables
    int connected = 1;
    int sockfd;
    char cSend[BUF];
    char cRecv[BUF];

    //- Create socket + Connect to server
    if ((sockfd = win32_connect(argc, argv, PORT)) < 0) {
        printf("Failed to connect to server.");
        exit(1);
    }
    ...
}

cNetwork.c:

#include "cHeader.h"

#ifdef LINUX
    ...irrelevant unix code...
#endif

#ifdef _WIN32
int win32_connect(int argc, char* argv[], int port)
{
    WSADATA wsaData;
    SOCKET ConnectSocket = INVALID_SOCKET;
    struct addrinfo *result = NULL,
        *ptr = NULL,
        hints;
    char *sendbuf = "this is a test";
    char recvbuf[BUF];
    int iResult;
    int recvbuflen = BUF;

    // Validate the parameters
    if (argc != 2) {
        //printf("usage: %s server-name\n", argv[0]);
        return 1;
    }

    // Initialize Winsock
    iResult = WSAStartup(MAKEWORD(2, 2), &wsaData);
    if (iResult != 0) {
        printf("WSAStartup failed with error: %d\n", iResult);
        return 1;
    }

    ZeroMemory(&hints, sizeof(hints));
    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_protocol = IPPROTO_TCP;

    // Resolve the server address and port
    iResult = getaddrinfo(argv[1], port, &hints, &result);
    if (iResult != 0) {
        printf("getaddrinfo failed with error: %d\n", iResult);
        WSACleanup();
        return 1;
    }

    // Attempt to connect to an address until one succeeds
    for (ptr = result; ptr != NULL; ptr = ptr->ai_next) {

        // Create a SOCKET for connecting to server
        ConnectSocket = socket(ptr->ai_family, ptr->ai_socktype,
            ptr->ai_protocol);
        if (ConnectSocket == INVALID_SOCKET) {
            printf("socket failed with error: %ld\n", WSAGetLastError());
            WSACleanup();
            return 1;
        }

        // Connect to server.
        iResult = connect(ConnectSocket, ptr->ai_addr, (int)ptr->ai_addrlen);
        if (iResult == SOCKET_ERROR) {
            closesocket(ConnectSocket);
            ConnectSocket = INVALID_SOCKET;
            continue;
        }
        break;
    }

    freeaddrinfo(result);

    if (ConnectSocket == INVALID_SOCKET) {
        printf("Unable to connect to server!\n");
        WSACleanup();
        return 1;
    }

    return ConnectSocket;
}
#endif
HyperionX
  • 1,332
  • 2
  • 17
  • 31
  • What line of code triggers the access violation? – Harry Johnston Sep 11 '15 at 02:29
  • amongst other problems with the code, `argc` is a simple integer. you can pass that integer all day, without ever taking the address of it. `argv[]` is already a pointer. you can pass that pointer all day, without ever taking the address of it. – user3629249 Sep 11 '15 at 19:00
  • this line: `#define _CRT_SECURE_NO_WARNINGS` before the 'include guard' will result in multiple definitions of that @define. In general, leading underscores should be completely avoided as the compiler prepends 1,2,or 3 leading underscores to all identifiers, depending on the compile step. Such 'user' supplied leading underscores in the source code can create confusion for the compiler. (although most modern compilers can handle that problem but why make things difficult) – user3629249 Sep 11 '15 at 19:04
  • you might want to lookup the definition of 'three star programmer' via google or wikipedia – user3629249 Sep 11 '15 at 19:06
  • the value BUF is not defined anywhere in the posted code. the variable `result` is not defined anywhere in the posted code. – user3629249 Sep 11 '15 at 19:20
  • 1
    the function: `win32_connect()` returns a '1' when the connection fails, however the `main()` function is looking for a returned value of <0 to indicate an connection failure. On success, that same function returns the value in: `ConnectSocket` which will be > 0. Suggest correct the interface between the functions. – user3629249 Sep 11 '15 at 19:25
  • regarding the layout of the code. I would strongly suggest checking the number of command line arguments back in the first part of the main() function then, if incorrect number of arguments is not correct exit. this would simplify the interface between the two functions as the `argc` parameter could be removed and keep the error checking (for number of command line arguments) to within the `main()` function rather than passed all through the code. Also, do not comment out the printf of the 'usage' statement as that is all that tells the user what is wrong – user3629249 Sep 11 '15 at 20:14
  • @user3629249 Thanks for your input, plus that 'three star programmer' is exactly what I DON'T want to be! It was a good read. Thanks for picking up on the win32_connect, that was an oversight I hadn't picked up yet. Also the _CRT_SECURE_NO_WARNINGS is only placed there as I'm using VS2015 and my assignment requires us to use VS2010. And some functions I would use are now deprecated and I cant compile without including that. BUF is defined in the header, I left it out as it was working fine. Thanks again for your comments, very helpful :) – HyperionX Sep 11 '15 at 23:27

1 Answers1

3

You don't need to pass pointers to argc and argv. Just declare the function like this:

int win32_connect(int argc, char *argv[], int port)

And you can use argc and argv normally. Then call it like this:

win32_connect(argc, argv, PORT);

EDIT:

You're passing an int for the second parameter to getaddrinfo. It expects a const char *. I don't see a definition for PORT, but I'm guessing it's a numerical constant.

So if you currently have (for example) this:

#define PORT 1234

Change it to this

#define PORT "1234"

Then define your function like this:

int win32_connect(int argc, char* argv[], const char *port)
dbush
  • 205,898
  • 23
  • 218
  • 273
  • That's what I had before posting this Q, just tried again and it still gives me : Exception thrown at 0x75D75616 (KernelBase.dll) in Project1.exe: 0xC0000005: Access violation reading location 0x00006987. If there is a handler for this exception, the program may be safely continued. – HyperionX Sep 11 '15 at 00:33
  • @Blake Then your issue is probably someplace else in your code. Post the full code and we'll see what's in there. – dbush Sep 11 '15 at 00:35
  • `argv = 0x012c93d0 {0x012c93dc "C:\\Users\\em-n-_000\\Desktop\\New folder (3)\\Project1\\Debug\\Project1.exe"}` `argv[1] = 0x012c9422 "localhost"` – HyperionX Sep 11 '15 at 00:35