Is it ok to use char* like a function type?

Question

I read about char*, that it should be used and instead of it I should use just char[]. Does it concern also function types? I paste below this what I read and my code below.

will place "Hello world" in the read-only parts of the memory, and making s a pointer to that makes any writing operation on this memory illegal What is the difference between char s[] and char *s?

char* GetModulePath()
{
    char ownPth[MAX_PATH];

    HMODULE hModule = GetModuleHandle(NULL);

    if (hModule != NULL)
    {
        GetModuleFileName(hModule, ownPth, (sizeof(ownPth)));
    }
    return ownPth;
}

So is it ok? Maybe instead of char* I should do it using const chars* ? EDIT: added link of this article which I've read.

You're returning a pointer to a local function variable, this is bad and has nothing to do with `char*` vs `char[]`. `ownPth` no longer exists once you return from `GetModulePath()`. — Borgleader, Mar 21 '18 at 13:03
`char*` and `char[]` are two different types. Maybe the question would be more clear if you provide a link to the place where read that. If it is about how to store strings, then just use `std::string` — 463035818_is_not_an_ai, Mar 21 '18 at 13:05
In C++, you *should* be using `std::string` except for interfacing into C APIs that require character arrays. — crashmstr, Mar 21 '18 at 13:05
@crashmstr except that `std::string` has a `c_str()` method, so you really never need raw char arrays as strings in c++ — 463035818_is_not_an_ai, Mar 21 '18 at 13:06
So should I change type to std::string, return also std::string and in scope of this funcion just convert from char to string? — , Mar 21 '18 at 13:08
@Michał Basically to stay out of most trouble, use `std::string` (or if using wide strings, `std::wstring`) for all string work until you're forced to send a pointer to a legacy function. There is no need to introduce `const char *` or `char *` coding when it is not necessary. — PaulMcKenzie, Mar 21 '18 at 13:37
@user463035818 except for when you need to pass an array of characters that the function will *modify* the contents, sure. There *are* ways of doing this - [C++ - Using STL Strings at Win32 API Boundaries](https://msdn.microsoft.com/en-us/magazine/mt238407.aspx), but they are not always as simple as `.c_str()` — crashmstr, Mar 21 '18 at 13:46

score 1 · Answer 1 · edited Mar 21 '18 at 14:53

1

Yes it is OK, but in your case you are returning a pointer to a local variable (mentioned in comments).

But anyway in C++ you just do this:

std::string GetModulePath()
{
    char ownPth[MAX_PATH] = {0};

    HMODULE hModule = GetModuleHandle(NULL);

    if (hModule != NULL)
    {
        GetModuleFileName(hModule, ownPth, sizeof(ownPth));
    }

    return std::string(ownPth);
}

edited Mar 21 '18 at 14:53

HatsuPointerKun

637
1
5
14

answered Mar 21 '18 at 13:14

Jabberwocky

48,281
17
65
115

HatsuPointerKun · Accepted Answer · 2018-03-21T14:50:44.150

It's ok to have a function with a char* return type, like that :

char* f() {
    char* r = new char[20];
    //Your logic
    return r;
}

int main() {
    char* v = f();
    //More logic
    //Don't forget to delete the dynamically allocated data when you don't need it
    delete[] v;
}

However, your code has a problem : you try to return the local variable ownPth, and that's a problem. In the line :

return ownPth;

you just return the pointer to the first element of the char array, which will be "destroyed" after the function call, and trying to dereference the returned value from outside will result in undefined behavior.

What you should do is using std::string like that :

std::string GetModulePath()
{
    char ownPth[MAX_PATH] = {0}; // Zero initialization

    HMODULE hModule = GetModuleHandle(NULL);

    if (hModule != NULL)
    {
        GetModuleFileName(hModule, ownPth, (sizeof(ownPth)));
    }
    return std::string(ownPth);
}

Or, if you really want to use char arrays, you should use heap allocation ( with new and delete ), but i don't recommand it :

char* GetModulePath()
{
    char* ownPth = new char[MAX_PATH];
    memset(ownPth,0,MAX_PATH); //Write zeroes in the allocated memory
    HMODULE hModule = GetModuleHandle(NULL);

    if (hModule != NULL)
    {
        GetModuleFileName(hModule, ownPth, MAX_PATH);
        //Not using sizeof, because now, ownPth is not a char array, it's a pointer
    }
    return ownPth; // You will need to delete[] it after, else it's a memory leak
}

As said in the comments below, it's very poor practice, and chances are, at some point, you will forget to call delete[] just after using it, after a function call, creating a memory leak

The last option is very, very poor practice. Don't do this ever. — Jabberwocky, Mar 21 '18 at 13:19
@MichaelWalz That's why i don't recommend that. Chances are that you will forget to `delete[]` somewhere after the call and create memory leak — HatsuPointerKun, Mar 21 '18 at 13:20
@HatsuPointerKun you should know about `std::unique_ptr` then. Manual memory management through `new` is an advanced topic. `std::unique_ptr` achieve the same but is substantially simply. Do yourself a favor and read about it. — Guillaume Racicot, Mar 21 '18 at 14:34
@HatsuPointerKun every time you forget a delete or type a new, you should put it a little upper in your list ;) — Guillaume Racicot, Mar 21 '18 at 14:49

Is it ok to use char* like a function type?

2 Answers2