C++ *LPCSTR weird scope error

Question

I currently have a class called TextureObject. In the creation function I create the texture, and assign a LPCSTR in the class to a parameter given in the function. When I return that LPCSTR later, it returns in an unexpected manner.

Some type names and functions are from DirectX 11, just ignore them.

Code:

The h File:

class TextureObject
{
public:
    ID3D11ShaderResourceView *pTexture;
    LPCSTR GetFilename() const { return *FFilename; }

    bool IsNotNull;
    void CreateTexture(ID3D11Device &dev,LPCSTR Filename);
    void ReCreate(ID3D11Device &dev);
    void Release();
    int relativeId;
private:
    LPCSTR *FFilename;
};

The cpp file:

void TextureObject::CreateTexture(ID3D11Device &dev,LPCSTR Filename)
{
    D3DX11CreateShaderResourceViewFromFile(
            &dev,        // the Direct3D device
            Filename,    // load Wood.png in the local folder
            NULL,        // no additional information
            NULL,        // no multithreading
            &pTexture,   // address of the shader-resource-view
            NULL);       // no multithreading
    FFilename = new LPCSTR(Filename);
    IsNotNull = true;
}

void TextureObject::ReCreate(ID3D11Device &dev)
{
    CreateTexture(dev, *FFilename);
}

When using vs 2012 debugger in the CreateTexture function, the Filename debugger values are:

0x0a06fed0 "C:\Users\Utilizador\Desktop\particle.png"

Which is perfect for me! When i assign the class's FFilename:

FFilename = new LPCSTR(Filename);

It's ok. When I check the value of FFilename within the scope of this function, it's the same value of the Filename. But when i use GetFilename, things start getting crazy:

 = 0x0a06fed0 "îþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþü =I.C"

Mmm, I just met you, and this is crazy, but... Here's my value. mKay?

Well, please help me. Thank You

score 2 · Answer 1 · answered Oct 23 '12 at 21:55

When you create your pointer FFilename, you're initializing it with another pointer. That's not going to make a copy of the string, now you have two pointers pointing to the same thing. Presumably that thing is a temporary object, and when you go to look at it later it's no longer valid.

I'd suggest using std::string for this instead, it's much less error prone. The c_str method can get a LPCSTR at any time.

score 2 · Accepted Answer · answered Oct 23 '12 at 21:56

2

You are not copying the string. You are copying the pointer. I think you probably wanted to copy the string, because you cannot guarantee the caller's pointer will still reference valid data at a later time.

LPCSTR is just a const char*. There's probably a corresponding windows call, but I would just use strdup to copy the string.

Define FFilename as LPCSTR:

LPCSTR FFilename;

And then:

void TextureObject::CreateTexture(ID3D11Device &dev,LPCSTR Filename)
{
    D3DX11CreateShaderResourceViewFromFile(
            &dev,        // the Direct3D device
            Filename,    // load Wood.png in the local folder
            NULL,        // no additional information
            NULL,        // no multithreading
            &pTexture,   // address of the shader-resource-view
            NULL);       // no multithreading
    FFilename = strdup(Filename);
    IsNotNull = true;
}

void TextureObject::ReCreate(ID3D11Device &dev)
{
    CreateTexture(dev, FFilename);
}

Since you are using C++, you are free to use std::string instead, which will be cleaned up automatically when the object is destroyed.

answered Oct 23 '12 at 21:56

paddy

60,864
6
61
103

So what i did wrong(Looking away from the pointer declaration) was the duplication. Thank you sir, and also the others... – Miguel P Oct 23 '12 at 21:57
Yes, the problem is you duplicated a pointer to a string instead of the string itself. If you use this method (instead of C++ strings) make sure you initialize `FFilename` to `NULL` in your constructor and `free` it in your destructor. – paddy Oct 23 '12 at 22:00
Downvoted! Perhaps the voter wishes to comment on what is wrong with this answer. – paddy Oct 23 '12 at 22:23
I didn't downvote, but see [this question](http://stackoverflow.com/questions/7582394/strdup-or-strdup) about `strdup`. – Jesse Good Oct 23 '12 at 23:07
@JesseGood Ahh okay... That would be splitting hairs I believe, considering `LPCSTR` and Direct3D are also not defined in the C standard. – paddy Oct 23 '12 at 23:33
Just remember there a lot of C++ purists lurking around. Also, you might want to mention that you are in charge of `free`ing the memory. – Jesse Good Oct 23 '12 at 23:38
Yep, had mentioned that in a comment. I think I'll just let it rest =) – paddy Oct 24 '12 at 00:45

score 1 · Answer 3 · answered Oct 23 '12 at 22:06

As marcin_j said, use std::[w]string. As for the line:

FFilename = new LPCSTR(Filename);

It just allocates 4 bytes for a pointer and initializes it to the filename string. It doesn't actually copy the string. So you can still use the string, but it is owned by whoever calls TextureObject::CreateTexture, and may be released while TextureObject is still referencing it.

Change the class to:

class TextureObject      
{      
public:      
    // ...all the same stuff as before...

private:      
    wstring FFilename;  // it's better to store filenames as Unicode
};

And the methods to:

void TextureObject::CreateTexture(ID3D11Device* dev, const wstring& Filename)        
{        
    D3DX11CreateShaderResourceViewFromFile(        
            dev,        // the Direct3D device        
            Filename.c_str(),    // load Wood.png in the local folder        
            NULL,        // no additional information        
            NULL,        // no multithreading        
            &pTexture,   // address of the shader-resource-view        
            NULL);       // no multithreading        

    FFilename = Filename;
    IsNotNull = true;        
}        

void TextureObject::ReCreate(ID3D11Device* dev)        
{        
    CreateTexture(dev, FFilename.c_str());        
}

C++ *LPCSTR weird scope error

3 Answers3