views:

76

answers:

3

Hello I have a pump class that requires using a member variable that is a pointer to a wchar_t array containing the port address ie: "com9".

The problem is that when I initialise this variable in the constructor my compiler flags up a depreciated conversion warning.

pump::pump(){
   this->portNumber = L"com9";}

This works fine but the warning every time I compile is anoying and makes me feel like I'm doing something wrong.

I tried creating an array and then setting the member variable like this:

pump::pump(){
   wchar_t port[] = L"com9";
   this->portNumber = port;}

But for some reason this makes my portNumber point at 'F'.

Clearly another conceptual problem on my part.

Thanks for help with my noobish questions.

EDIT:

As request the definition of portNumber was:

    class pump
{
private:
   wchar_t* portNumber;
}

Thanks to answers it has now been changed to:

    class pump
{
private:
   const wchar_t* portNumber;
}
+6  A: 

If portNumber is a wchar_t*, it should be a const wchar_t*.

String literals are immutable, so the elements are const. There exists a deprecated conversion from string literal to non-const pointer, but that's dangerous. Make the change so you're keeping type safety and not using the unsafe conversion.

The second one fails because you point to the contents of a local variable. When the constructor finishes, the variable goes away and you're pointing at an invalid location. Using it results in undefined behavior.

Lastly, use an initialization list:

pump::pump() :
portNumber(L"com9")
{}

The initialization list is to initialize, the constructor is to finish construction. (Also, this-> is ugly to almost all C++ people; it's not nice and redundant.)

GMan
+2  A: 

Use const wchar_t* to point at a literal.

The reason the conversion exists is because it has been valid from early versions of C to assign a string literal to a non-const pointer[*]. The reason it's deprecated is that it's invalid to modify a literal, and it's risky to use a non-const pointer to refer to something that must not be modified.

[*] C didn't originally have const. When const was added, clearly it should apply to string literals, but there was already code out there, written before const existed, that would break if suddenly you had to sprinkle const everywhere. We're still paying today for that breaking change to the language. Since it's C++ you're using, it wasn't even a breaking change to this language.

Steve Jessop
One of the things that ruin most C++ is the C backwards compatibility even in the most idiotic choices (C standard library included).
Matteo Italia
+1  A: 

Apparently, portNumber is a wchar_t * (non-const), correct? If so:

  • the first one is wrong, because string literals are read-only (they are const pointers to an array of char usually stored in the string table of the executable, which is mapped in memory somewhere, often in a readonly page).
    The ugly, implicit conversion to non-const chars/wchar_ts was approved, IIRC, to achieve compatibility with old code written when const didn't even existed; sadly, it let a lot of morons which do not know what const correctness means get away with writing code that asks non-const pointers even when const pointers would be the right choice.

  • The second one is wrong because you're making portNumber point to a variable allocated on the stack, which is deleted when the constructor returns. After the constructor returns, the pointer stored in portNumber points to random garbage.

The correct approach is to declare portNumber as const wchar_t * if it doesn't need to be modified. If, instead, it does need to be modified during the lifetime of the class, usually the best approach is to avoid C-style strings at all and just throw in a std::wstring, that will take care of all the bookkeeping associated with the string.

Matteo Italia
Thanks for the info. In this case the port number may be changed dynamically (from a drop down menu) because the pump may not always be on com9. The reason I'm using a wchar_t pointer at all is because it is required as an argument in the CreateFile() function. I will see if it accepts a std::wstring.
Zac
It doesn't accept a wstring, but wstring (as any string built on std::basic_string) provides the c_str() method that returns a const wchar_t * to let C functions access its content.
Matteo Italia