allow literal string only

[OReubens]

is it possible to create a constructor that takes as parameter ONLY a literal string?

I have a class that is designed to be used with a literal string only. This requirement allows us to simplify what the constructor does by assumption that we can 'store' the string only by copying the pointer, without having to copy the contents of the string.

class Foo
{
public:
   Foo(LPCSTR lpsz) m_str(lpsz) {}

   LPCSTR m_str;
}

This requirement can't be changed, this is both a performance issue, and security issue in that the string is assumed to be located in readonly memory.

There's currently a big warning plastered on the function prototype saying to use it ony with a literal, but this has been "overlooked" several times in the past resulting in hard to track down issues.

Ideal would be if we can get the compiler to simply reject everything other than a literal.

so in short:

Foo("this is ok");

static const char sz[] = "this is also ok";
Foo(sz);

char sz2[] = "Modifiable strings are not ok";
Foo(sz2); // should give compiler error

char sz3[255];
strcpy(sz3, "C-style string is not ok");
Foo(sz3); // should give compile error

CString str("MFC string is not ok");
Foo(str); // should give compile error

std::string str2("STL string is not ok");
Foo(str2); // should give compile error

Is this possible ?

[Skizmo]

Is this possible ?

AFAIK, no. A char pointer is a char pointer... nothing more, nothing less. Doesn't matter where it comes from, it is still a char pointer.

[TheComputer]

Human beings are different from machine, this is about programming right ?

it's type mismatches, coders will cast them themselves or just feel pleased to use default settings by MSVC IDE (no more use of "sha" from the start)

[GCDEF]

Human beings are different from machine, this is about programming right ?

it's type mismatches, coders will cast them themselves or just feel pleased to use default settings by MSVC IDE (no more use of "sha" from the start)

What?

[laserlight]

Ideal would be if we can get the compiler to simply reject everything other than a literal.

Is this possible ?

I agree with Skizmo that this sounds impossible. However, you could cut down on possible careless mistakes by overloading for a constructor that takes an LPSTR, and then use an assertion to check that this constructor is never invoked. It won't detect cases where the constructor is invoked with an array of const char that is not a string literal, of course, but it would detect two of those cases that you listed. (The last two cases should already be compile errors, right?)

AFAIK, no. A char pointer is a char pointer... nothing more, nothing less. Doesn't matter where it comes from, it is still a char pointer.

That said, in terms of type a string literal is not a char pointer, but an array, which is converted to a char pointer. But your reasoning seems sound to me otherwise. I considered using this fact with a template constructor that takes an array by reference, but passing a pointer could be the right thing to do if that pointer points to the first element of a string literal, and it still will not do anything about arrays of const char that are not string literals, as you reasoned.

[S_M_A]

I played around alittle and at least in VC2008 you can get a bit more safety by adding a Foo(char*) with nothing more than an assert in the body. sz2 & sz3 will be catched but not all variants for CString or std::string since the access operators return a const char*.

However, some additional safety is better than nothing I guess?

Edit: Thanks laserlight for saving me the need of double-checking the standard to back up my post...

[superbonzo]

as other said it seems impossible with the use of the sole type system ( unless compiler intrisics exist enabling static storage detection ... );

maybe you can use the preprocessor somehow, something like

#define LITERAL(s) Foo::please_dont_use_me(""s"")
#define LITERAL_TYPE please_dont_use_me

class Foo
{
public:
	struct please_dont_use_me
	{
		explicit please_dont_use_me( const char* str ): value(str) {}

		const char* value;
	};

	Foo( LITERAL_TYPE str ): str_( str.value ) {}

private:
	const char* str_;
};

Foo x( LITERAL("ok") );

in this way, if you construct a Foo with something that has not the form LITERAL("...") an error should occur ( more specifically, it seems improbable someone accidentally instances a Foo with a Foo :: please_dont_use_me object ... ).

EDIT: thinking about it, the following might be more expressive ( but roughly equivalent ):

#define LITERAL(s) Foo::please_dont_use_me(""s"")

class Foo
{
	struct literal
	{
		explicit literal( const char* str ): value(str) {}

		const char* value;
	};

public:
	typedef literal please_dont_use_me;

	Foo( literal str ): str_( str.value ) {}

private:
	const char* str_;
};

Foo x( LITERAL("ok") );

BTW, note that it wouldn't protect you against expressions like "Foo x( LITERAL("ok"+"oops") );" and similar ...

[ahmd]

As pointed above, there's no way to detect this at a compilation time, but it doesn't mean that you can't do it at a runtime though. As far as I know the latest Operating Systems (since Vista) are employing a different (i.e. more secure) memory management strategy, as well as a different approach to loading executables. And, this means that you can use it for the purpose of your question. So the whole idea is to test whether a string is writable in memory and if so, it will mean that it is not a static string. In other words, anything that is not declared static is allocated either on the stack or from a heap and is writable.

OK, so here's the code:

Add the following in the declaration of one of your globally accessible classes:

	static __forceinline void DebuggerTestConstStringPtr(LPCTSTR pString, int nCodeLine, LPCSTR pCodeFile)
	{
//	#ifdef _DEBUG
		__try
		{
			BOOL bNonWritableMem = ::IsBadWritePtr((LPVOID)pString, (lstrlen(pString) + 1) * sizeof(TCHAR));

			ASSERT(bNonWritableMem == TRUE);	//Assertion here means you passed a non-static string
			if(bNonWritableMem != TRUE)
			{
				TRACE(_T("Non-static string passed on line %d in %s\n"), nCodeLine, pCodeFile);
			}
		}
		__except(1)
		{
			ASSERT(FALSE);	//Bad string pointer
			TRACE(_T("Bad string passed on line %d in %s\n"), nCodeLine, pCodeFile);
		}
//	#endif
	}

and then you can simply call this method from anywhere in your code:

	static const TCHAR sz[] = _T("this is also ok");

	CGlobalClass::DebuggerTestConstStringPtr(sz, __LINE__, __FILE__);
	CGlobalClass::DebuggerTestConstStringPtr(_T("This is OK"), __LINE__, __FILE__);

	TCHAR sz2[] = _T("this is not ok");
	CGlobalClass::DebuggerTestConstStringPtr(sz2, __LINE__, __FILE__);   //ASSERTs!

	CString str("MFC string is not ok");
	CGlobalClass::DebuggerTestConstStringPtr(str, __LINE__, __FILE__);   //ASSERTs!

	std::string str2("STL string is not ok");
	CGlobalClass::DebuggerTestConstStringPtr(str2.c_str(), __LINE__, __FILE__);   //ASSERTs!

NOTE that you have to run it on Vista or Windows 7 (and possibly compile with a newer version of Visual Studio) to be able to track writable memory. It is also not advisable to keep the IsBadWritePtr API in your distribution build, since that API is not safe (read MSDN for more into). You may also want to skip using the /NXCOMPAT, and /DYNAMICBASE compiler directives.

[OReubens]

Ahmd:
The runtime check is unacceptable because of performance issues. Checking for a read segment is considerably more of a performance penalty than copying the string contents, and even that is already too much of an issue. This solution also means we need to specifically test each instance (which I'm trying to avoid), and can't suffise with a single unit-test.

Superbonzo:
interesting twist, but it doesn't work. For one it requires the caller to explicitely use the macro. Which means we'd have to change a LOT of existing code, meaning a lot of regression testing as well. For two , nothing prevents a programmer from writing:
Foo x( LITERAL(strMfcString)); // or stl string, or buffer or anything that has a conversion to LPCSTR.

Skizmo:
A char pointer is a char pointer. But I'm interested in a way to make the function so it does NOT accept a char pointer. LPCSTR is too generic for what i'm looking for, I need a more specific 'something', that isn't a char pointer.
FWIW as laserlight pointed out, a literal string is not a char pointer, it just gets implicitely converted to a char pointer by the compiler to match the function prototype. I can even pass any object I want as a parameter to Foo, as long as that object has a LPCSTR cast operator.

class Bar
{
public:
	Bar() { }
	operator LPCSTR() { return "Bar"; }
};

Bar b;
Foo(b); // Valid with Foo taking a LPCSTR parameter

SMA: Adding a char* variant would again mean extra regression testing and requirement to test each instance. But it does pop an idea into my head... if the char* constructor is private, and there isn't some C++ rule to make the compiler fall back to another constructor if the preferred one is private. Will be doing some investigating in that direction...

[superbonzo]

nothing prevents a programmer from writing:
Foo x( LITERAL(strMfcString)); // or stl string, or buffer or anything that has a conversion to LPCSTR.

no, LITERAL(strMfcString) expands to Foo:: please_dont_use_me(""strMfcString"") which is a compiler error ( whereas LITERAL("ok") expands to Foo:: please_dont_use_me("""ok""") that in turn is legally interpreted as simply Foo:: please_dont_use_me("ok") ... )

anyway, regarding your first objection I've nothing to say

[OReubens]

I think I solved it

class Foo
{
public:
	// This constructor will only accept character references with a known compile-time size.
	template <std::size_t size>
	Foo(const char (&str)[size])
	{
		m_cp = str;
	}
private:
	func (char*  lpsz)  // this private constructor, blocks the use of c-style character buffers.
	{
	}

	const char* m_cp; 
};

with the above:

Foo f("Literal"); // this works

const char s1[] = "s1";
Foo f1(s1); // This works

static const char s2[] = "s2";
Foo f2(s2); // This works

Everything else i've tried so far fails (as desired). Either because there's no matching constructor, or because the char* constructor is private.

It does mean the above is restricted to member functions where you can make a member private. It won't work on global functions.

Anyone notice anything I missed with the above ?

[laserlight]

I think I solved it
(...)
Anyone notice anything I missed with the above ?

Oh, that is precisely what I meant by:

I considered using this fact with a template constructor that takes an array by reference, but passing a pointer could be the right thing to do if that pointer points to the first element of a string literal, and it still will not do anything about arrays of const char that are not string literals, as you reasoned.

If you are willing to consider this correct:

const char s1[] = "s1";
Foo f1(s1); // This works

then yes, there is no problem. But the above is suspect, in my opinion, as the member pointer would then be pointing to the first char of a string that may well be a local variable that goes out of scope, unlike a string literal or static const char array/pointer. Are you sure that this is desired?

[OReubens]

If you are willing to consider this correct:

const char s1[] = "s1";
Foo f1(s1); // This works

then yes, there is no problem. But the above is suspect, in my opinion, as the member pointer would then be pointing to the first char of a string that may well be a local variable that goes out of scope, unlike a string literal or static const char array/pointer. Are you sure that this is desired?

Ah *bleep*...

Didn't consider that being used from inside a function. Not being static causes the compiler to "allocate" this buffer on the stack and copying the contents to it from the readonly memory to the stack. When the function returns, the value is gone.

In this case specifically, it's acceptable. For one, the scope of the 'Foo' object is always the same function that could possibly have that string definition.
And for 2nds, non-static (and non-const) arrays defined at function scope are marked as suspicious by our build tools. This means getting this to pass requires the developer to provide a proof that his solution is acceptable and doesn't cause unneccesary performance issues. In several million lines of code, we only have 2 such cases so far.

Would be nice to catch that case specifically also to have a more generic solution to the issue.

[OReubens]

Hmmm I'm actually confused as to why this difference (having or not having 'static') actually matters on a const at function scope...

You're telling the compiler the char array is const (at function scope). But why does the compiler go through the trouble of making a local copy since you've said it's const anyway. I'll agree that there is the case where you could cast away the const and overwrite the string anyway, but that seems like a very poor reason to cause an unnecessary copy. Or am I missing something?

[laserlight]

But why does the compiler go through the trouble of making a local copy since you've said it's const anyway.

It might not, but then you would be depending on a compiler optimisation that is not guaranteed to happen.

I'll agree that there is the case where you could cast away the const and overwrite the string anyway

Overwriting the string through the non-const pointer (from the const_cast) would result in undefined behaviour, and I believe that this is precisely because the standard committee had such an optimisation in mind.