0

I'm trying to convert a vector of string to a char ** to pass into another method that makes a hostent struct. I have been trying to figure this out, but don't know what to do. Here's what I am doing

#include <stdlib.h>
#include <sys/types.h>
#include <string.h>
#include <stdio.h>
#include <iostream>
#include <fstream>
#include <netdb.h>
#include <vector>
#include <algorithm>
#include <iterator>

using namespace std;

static struct hostent host;

#define INADDRSZ 4
#define _UNCONST(a) ((void *)(unsigned long)(const void *)(a))

void tokenize(string const &str, const char delim, vector<std::string> &out) {
    size_t start;
    size_t end = 0;

    while ((start = str.find_first_not_of(delim, end)) != std::string::npos){
        end = str.find(delim, start);
        out.push_back(str.substr(start, end - start));
    }
}

static struct hostent * mk_hostent (const char *name, char **aliases, char **haddress, int addrsz, int af) {
    host.h_name = const_cast<char*> (name);
    host.h_addr_list = haddress;
    host.h_aliases = aliases;
    //host.h_addr_list[index] = (char *)_UNCONST(&host_addr[index]);
    host.h_length = addrsz;
    host.h_addrtype = af;
    //host.h_aliases = _UNCONST(aliases);
    return (&host);
}

void getHostAddrList (string hostname, vector<string> ipsAndHost, vector<string> &addrs) {
    cout << "I reached here" << "\n";
    vector<string> tokens;
    for (int i = 0; i < ipsAndHost.size(); i++) {
        tokenize(ipsAndHost[i], ' ', tokens);
        // comparing if hostname same then pushing ip to host addrs list
        if (hostname.compare(tokens[1]) == 0) {
            addrs.push_back(tokens[0]);
        }
        tokens.clear();
    }
}

void getAliasList (string hostip, vector<string> ipsAndHost, vector<string> &aliases) {
    vector<string> tokens;
    for (int i = 0; i < ipsAndHost.size(); i++) {
        tokenize(ipsAndHost[i], ' ', tokens);
        // comparing if host ip same then pushing host alias to aliases list
        if (hostip.compare(tokens[0]) == 0) {
            aliases.push_back(tokens[1]);
        }
        tokens.clear();
    }
}


char ** vector2string (vector<string> &input) {
    char ** arr = new char*[input.size()];
    for(size_t i = 0; i < input.size(); i++){
        arr[i] = new char[input[i].size() + 1];
        strcpy(arr[i], input[i].c_str());
    }
    return arr;
}

static struct hostent * gethostbyaddr_iri (const char *addr, int af) {
//void printLines(const char * addr){
    fstream file("sampleFile");
    string line;
    vector<string> readLines;

    while (getline(file, line)) {
        // skipping empty line
            if (line.empty()) {
                continue;
            }
        // skipping comments
        if (line[0] == '#') {
            continue;
        }

        readLines.push_back(line);

    }

    vector<string> ips;
    vector<string> hostAddresses;
    vector<string> hostAliases;

    for (int i = 0; i < readLines.size(); i++) {
        tokenize(readLines[i], ' ', ips);
        //ips[0] will have ip address to compare
        if (strcmp(ips[0].c_str(), addr) == 0) {
            cout << "one was same" << "\n";
            getHostAddrList(ips[1], readLines, hostAddresses);
            getAliasList(ips[0], readLines, hostAliases);
            //return mk_hostent(ips[1],
        }
        // clearing vector for next loop
        ips.clear();

        char ** hAddr = vector2string(hostAddresses);
        char ** hAlias = vector2string(hostAliases);

        return mk_hostent(ips[1].c_str(), hAlias, hAddr, INADDRSZ, AF_INET);
    }

}

int main() {
    gethostbyaddr_iri("128.0.0.16", 2);
}

So if you see I call the vector2string method to convert my vector string into char ** that I then pass to the mk_hostent method. The memory is never being freed and I need a way to convert my vectors to char ** without causing this memory leak

shafi97
  • 21
  • 1
  • 5
  • Why are you not using `std::vector>` in `mk_hostent`? – R Sahu Nov 22 '19 at 18:30
  • hostent is a linux thing and it requires char ** for those fields – shafi97 Nov 22 '19 at 18:31
  • 1
    Then free the memory you allocated. I don't understand your question (is there a question?). You state you don't free memory. So do it. Also aliases and addr_list should be null terminated, you're aren't (at least I don't see where). – KamilCuk Nov 22 '19 at 18:31
  • 1
    If you really have to do things this way, use a `std::vector` ... but I'm not convinced your general approach is at all meaningful. – o11c Nov 22 '19 at 18:32
  • I can't free it because I return the object then i'm out of the method. So I need another way to do this – shafi97 Nov 22 '19 at 18:33
  • Then don't return it and free it? Return a custom object with a custom destructor that encapsulates hostent? Or just return `std::unique_ptr` with a custom deleter? – KamilCuk Nov 22 '19 at 18:34
  • I need the thing thats being returned – shafi97 Nov 22 '19 at 18:34
  • Then the consumer of "the thing that's being returned" is responsible for freeing the object and allocated memory. `example`: [How do I use a custom deleter with a std::unique_ptr member?](https://stackoverflow.com/questions/19053351/how-do-i-use-a-custom-deleter-with-a-stdunique-ptr-member) – KamilCuk Nov 22 '19 at 18:35
  • @KamilCuk can you give an example how you would do that – shafi97 Nov 22 '19 at 18:35
  • @shafi97 `static struct hostent host;` -- If you were willing to make this `static`, why not the same for the `vector`(s)? Then there is no issue with [vector lifetime](https://coliru.stacked-crooked.com/a/834769c04406dea5) – PaulMcKenzie Nov 22 '19 at 18:48
  • Currently, `main()` calls `gethostbyaddr_iri()` and discards its return value. Nothing else is done with the returned pointer. But that also means that your code, as shown, has gone to the trouble of allocating memory that is never used. If your intent is to be able to modify `main()` so it uses the pointer returned `gethostbyaddr_iri()` - rather than discarding it - then subsequent usage of that pointer in `main()` means that the allocated memory is still needed. In other words, in the code you have shown, there is not yet a place where the allocated memory CAN be safely released. – Peter Nov 22 '19 at 18:56

1 Answers1

1

All you need is an array of char* to hold the addresses of the various strings. To avoid memory leaks you can use std::vector<char*> to point into your vector of strings like this:

std::vector<char*> vector2string (std::vector<string>& input) {

    std::vector<char*> arr;

    for(auto& s: input)
        arr.push_back(&s[0]); // add the address of each string array

    return arr;
}

And use it like this:

std::vector<char*> hAddr = vector2string(hostAddresses);
std::vector<char*> hAlias = vector2string(hostAliases);

hostent host = mk_hostent(ips[1].c_str(), hAlias.data(), hAddr.data(), INADDRSZ, AF_INET);

Be sure that hAddr and hAlias are unused after hostAddresses & hostAliases respectively are changed because they point into the live data. The same goes for the returned host object which has pointers to their data.

Galik
  • 47,303
  • 4
  • 80
  • 117
  • 2
    BTW, this is one of the few places `std::vector` makes sense. – PaulMcKenzie Nov 22 '19 at 19:27
  • 2
    `mkhostent()` returns a pointer to a `struct hostent` that, in turn, contains the pointers `hAlias.data()` and `hAddr.data()` that are passed here. `hAddr` and `hAlias` go out of scope as the function returns. The caller of this code therefore receives a pointer to `struct hostent` that contains dangling references. – Peter Nov 22 '19 at 20:32