class CFoo
{
public:
CFoo(LPCTSTR lpszText) { Set(lpszText); }
~CFoo() {}
LPCTSTR Get() const { return m_szText; }
void Set(LPCTSTR lpszText) {
strncpy(m_szText, lpszText, sizeof(m_szText));
}
private:
char m_szText[256];
};
int main()
{
CFoo foo("My text");
LPCTSTR lpszCantChange = foo.Get();
*lpszCantChange = NULL; // COMPILATION ERROR. GOOD.
char* lpszCanChange = (char*)foo.Get();
*lpszCanChange = NULL; // BREAKS DATA HIDING
}
You see the problem. Even though m_szText is private, it is still
possible to alter its value and so the state of CFoo's instance. You
cannot do
strncpy(foo.m_szText, "New Text", sizeof(foo.m_szText)) but above
'loophole' would break object-orientedness.
Only way to prevent this is to make a copy of it and return it, as far
as I know. Does somebody know how to resolve this without copying it?
-Thanks.
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Hmmm. Using the cast, you said "hey compiler, stop complaining about the
constness because I know better!" and the compiler does so. That means you
are now responsible for treating the data properly, which you fail to do.
One more reason not to use those casts.
> Only way to prevent this is to make a copy of it and return it, as far
> as I know. Does somebody know how to resolve this without copying it?
I don't think that's worth the hassle. C++ protects you from accidentially
violating restrictions, but what you showed here is deliberate vandalism.
One last note: a TCHAR (the base for LPCTSTR) is not necessarily the same as
a char. Your program will not compile and run on systems where e.g.
TCHAR=wchar_t (e.g. on WinCE). But that is mostly off-topic here. I won't
even mention some other problems that probably stem from this being just an
example, but there are some.
Uli
--
FAQ: http://parashift.com/c++-faq-lite/
/* bittersweet C++ */
default: break;
>Look at this sample:
>
>class CFoo
>{
>public:
> CFoo(LPCTSTR lpszText) { Set(lpszText); }
> ~CFoo() {}
>
> LPCTSTR Get() const { return m_szText; }
> void Set(LPCTSTR lpszText) {
> strncpy(m_szText, lpszText, sizeof(m_szText));
> }
>
>private:
> char m_szText[256];
>};
>
>int main()
>{
> CFoo foo("My text");
> LPCTSTR lpszCantChange = foo.Get();
> *lpszCantChange = NULL; // COMPILATION ERROR. GOOD.
>
> char* lpszCanChange = (char*)foo.Get();
> *lpszCanChange = NULL; // BREAKS DATA HIDING
How? If someone casts away constness they had better know what they
are doing, and if they break something they have only themselves to
blame. Note you shouldn't generally use C-style casts in C++ code.
Instead:
char* lpszCanChange = const_cast<char*>(foo.Get());
That makes it much clearer what's happening.
>}
>
>You see the problem. Even though m_szText is private, it is still
>possible to alter its value and so the state of CFoo's instance.
Yes, there are many ways to alter it "illegally". e.g.
CFoo foo("My text");
char* ptr = reinterpret_cast<char*>(&foo);
*ptr = '\0';
But so what?
You
>cannot do
>strncpy(foo.m_szText, "New Text", sizeof(foo.m_szText)) but above
>'loophole' would break object-orientedness.
Why? If someone really wants to modify the memory in your object there
are many ways of doing it - C++ doesn't have a protection model like
Java; instead you have the power to do what you want, but 'with power
comes responsibility'.
>Only way to prevent this is to make a copy of it and return it, as far
>as I know. Does somebody know how to resolve this without copying it?
Even copying it doesn't solve the "problem". But why do you think it
is a problem?
Tom
--
C++ FAQ: http://www.parashift.com/c++-faq-lite/
C FAQ: http://www.eskimo.com/~scs/C-faq/top.html
Yes: the problem is that your coding standard allows C-style casts. ;-)
> Even though m_szText is private, it is still possible to alter its
> value and so the state of CFoo's instance.
<snip>
Yes, and that can be done by writing "*(char *)&foo = 0;" as well.
There's nothing you can do to prevent wilful violation of
encapsulation in another part of the program.
> class CFoo
> {
> public:
> CFoo(LPCTSTR lpszText) { Set(lpszText); }
> ~CFoo() {}
> LPCTSTR Get() const { return m_szText; }
> void Set(LPCTSTR lpszText) {
> strncpy(m_szText, lpszText, sizeof(m_szText));
> }
What's an "LPCTSTR"?
> char* lpszCanChange = (char*)foo.Get();
> *lpszCanChange = NULL; // BREAKS DATA HIDING
You're casting here. Even though C++ helps in data hiding, it doesn't
prevent you from shooting yourself in your foot. The "getter" function
should probably either return a constant pointer to a constant string,
or should return a string object right away. If that's implemented
nicely, it won't copy anything unless required.
> Only way to prevent this is to make a copy of it and return it, as far
> as I know. Does somebody know how to resolve this without copying it?
Use a wrapper object for a string that implements a "copy on write". I'd
guess popular string implementations might even do that. Or don't cast -
if you play bad with the language, what do you expect will happen?
So long,
Thomas
> Look at this sample:
>
> class CFoo
> {
> public:
> CFoo(LPCTSTR lpszText) { Set(lpszText); }
> ~CFoo() {}
>
> LPCTSTR Get() const { return m_szText; }
> void Set(LPCTSTR lpszText) {
> strncpy(m_szText, lpszText, sizeof(m_szText));
> }
>
> private:
> char m_szText[256];
> };
Why do you use char* instead of std::string? Pointers are bad, so
try to avoid them in C++ whereever you can.
> int main()
> {
> CFoo foo("My text");
> LPCTSTR lpszCantChange = foo.Get();
> *lpszCantChange = NULL; // COMPILATION ERROR. GOOD.
>
> char* lpszCanChange = (char*)foo.Get();
C-Style casts are bad (you're casting the const away here). To solve this
problem, use a C++ style cast.
> *lpszCanChange = NULL; // BREAKS DATA HIDING
Surely it does. You expilictly casted away the const. What did you
expect?
Resumee: If you mix up C++ and C too much, you don't have to wonder
if something bad happens. Sticking to C++ will solve the problem
in an easy and elegant way.
--
To get my real email adress, remove the two onkas
--
Dipl.-Inform. Hendrik Belitz
Central Institute of Electronics
Research Center Juelich
I do wish you would sanitize your code samples by removing all the
Hungarian stuff which only serves to obfuscate it.
>
>class CFoo
>{
>public:
> CFoo(LPCTSTR lpszText) { Set(lpszText); }
> ~CFoo() {}
>
> LPCTSTR Get() const { return m_szText; }
> void Set(LPCTSTR lpszText) {
> strncpy(m_szText, lpszText, sizeof(m_szText));
> }
>
>private:
> char m_szText[256];
>};
>
>int main()
>{
> CFoo foo("My text");
> LPCTSTR lpszCantChange = foo.Get();
> *lpszCantChange = NULL; // COMPILATION ERROR. GOOD.
>
> char* lpszCanChange = (char*)foo.Get();
> *lpszCanChange = NULL; // BREAKS DATA HIDING
>}
>
>You see the problem. Even though m_szText is private, it is still
>possible to alter its value and so the state of CFoo's instance. You
>cannot do
>strncpy(foo.m_szText, "New Text", sizeof(foo.m_szText)) but above
>'loophole' would break object-orientedness.
>
>Only way to prevent this is to make a copy of it and return it, as far
>as I know. Does somebody know how to resolve this without copying it?
Anytime you use a cast (and particularly a C-style one) you take
responsibility for the consequences. Your code above is the moral
equivalent of theft because you deliberately circumvent the compiler's
normal processes of protection. It is a kind of breaking and entering.
Now had you used C++ casts the compiler could be more helpful:
char * CanChange = static_cast<char *>foo.Get();
I think fails because you are not allowed to cast away 'const' with a
static_cast<>. Using const_cast<> would make the breaking in obvious and
immediately suspect in the eyes of any code reviewer.
--
Francis Glassborow ACCU
Author of 'You Can Do It!' see http://www.spellen.org/youcandoit
For project ideas and contributions: http://www.spellen.org/youcandoit/projects
You could also do a memcpy(&foo,"destroy the object",10) - the purpose of
private/public sections is not to stop crooks but to stop accidental abuse.
>
> Only way to prevent this is to make a copy of it and return it, as far
> as I know. Does somebody know how to resolve this without copying it?
No - when casts are made anything might happen. Avoid casts whenever you can
and in particular avoid C-style casts as they are imprecise and dangerous.
Now, I do not know what "LPCTSTR" but i guess it corresponds to a char
const*. If this is the case, you have already provided enough protection and
the user of your class is to blame. If not change the signature of get to
char const* Get().
/Peter
"ps" <ps...@nextgen.com> wrote
> Look at this sample:
>
> class CFoo
> {
> public:
> CFoo(LPCTSTR lpszText) { Set(lpszText); }
> ~CFoo() {}
>
> LPCTSTR Get() const { return m_szText; }
> void Set(LPCTSTR lpszText) {
> strncpy(m_szText, lpszText, sizeof(m_szText));
> }
>
> private:
> char m_szText[256];
> };
>
> int main()
> {
> CFoo foo("My text");
> LPCTSTR lpszCantChange = foo.Get();
> *lpszCantChange = NULL; // COMPILATION ERROR. GOOD.
>
> char* lpszCanChange = (char*)foo.Get();
> *lpszCanChange = NULL; // BREAKS DATA HIDING
> }
>
> You see the problem. Even though m_szText is private, it is still
> possible to alter its value
First you use a C style cast, and then you complain that you can do anything
with the result? You use a proprietary typedef (that is LPCTSTR, as I
googled) without explanation, and ask us how to use it properly? That is a
bit bold.
If you are implying that C++ is an inherently unsafe/insecure language (e.g.
it shows implementation details, and you can cast away constness): You are
right.
But even in in a language with security as a primary design goal, like Ada,
you could use an unchecked_conversion and do any damage you like.
It is your own responsibility to follow a good coding style and to know what
your code is doing.
Regards, Peter
// For those of you who do not use Microsoft Windows:
// LPCTSTR is a typedef for either const char* or const wchar_t*
> CFoo(LPCTSTR lpszText) { Set(lpszText); }
> ~CFoo() {}
>
> LPCTSTR Get() const { return m_szText; }
// Note that Set() doesn't work well if LPCTSTR is const whcar_t* !
> void Set(LPCTSTR lpszText) {
> strncpy(m_szText, lpszText, sizeof(m_szText));
> }
> private:
> char m_szText[256];
> };
>
> int main() {
> CFoo foo("My text");
> LPCTSTR lpszCantChange = foo.Get();
> *lpszCantChange = NULL; // COMPILATION ERROR. GOOD.
// Note the cast you needed here!
> char* lpszCanChange = (char*)foo.Get();
> *lpszCanChange = NULL; // BREAKS DATA HIDING
> }
Because LPCTSTR is points to const characters, this wouldn't have worked
without a cast. Half of the casts used in Microsoft sample code aren't
needed, and this particular one is dangerous. Try writing the same code
without the cast and you'll see the message:
error C2440: 'initializing' :
cannot convert from 'const char *' to 'char *'
Conversion loses qualifiers
> Only way to prevent this is to make a copy of it and return it, as far
> as I know. Does somebody know how to resolve this without copying it?
Make and enforce a new rule: never use any type of cast unless you can
defend it in a code review.
"ps" <ps...@nextgen.com> wrote...
>
> int main()
> {
> CFoo foo("My text");
> LPCTSTR lpszCantChange = foo.Get();
> *lpszCantChange = NULL; // COMPILATION ERROR. GOOD.
>
> char* lpszCanChange = (char*)foo.Get();
> *lpszCanChange = NULL; // BREAKS DATA HIDING
> }
I don't think this breaks data hiding. You cast away the const. That
should sound alarm bells. The CFoo class can't protect itself against
this. You could equally have written...
memset( &foo, 0, sizeof(foo) );
...which also "breaks encapsulation", but using memset with an object
should sound alarm bells too.
In fact, anything involving...
any kind of cast except dynamic_cast
void* (such as the memset family of functions)
...can cause similar trouble. The C++ type system defends against
accident, not fraud.
If someone goes out of their way to cast away the constness of a
pointer, they'd better have a very good reason and know exactly what
they are doing, and there isn't much you can do about it.
Your class "Foo" uses the correct method for properly allowing read
access to the m_szText member. If someone explicitly circumvents that,
and has problems because of it, it is their fault, not yours.
Even if you did do something like making and returning a copy, someone
could still go through some pointer gymnastics to get at the m_szText
member. For example, on most platforms/compilers you could get it
simply using reinterpret_cast<char *>(&foo).
Do you have a particular reason you need to protect against explicit misuse?
Alan
> Look at this sample:
>
> class CFoo
> {
> public:
> CFoo(LPCTSTR lpszText) { Set(lpszText); }
Note the pointer to the data is copied from an outside
source. Therefor, the data pointed is availaible to other code
anyway.
> ~CFoo() {}
>
> LPCTSTR Get() const { return m_szText; }
> void Set(LPCTSTR lpszText) {
> strncpy(m_szText, lpszText, sizeof(m_szText));
> }
>
> private:
> char m_szText[256];
> };
>
> int main()
> {
> CFoo foo("My text");
> LPCTSTR lpszCantChange = foo.Get();
> *lpszCantChange = NULL; // COMPILATION ERROR. GOOD.
>
> char* lpszCanChange = (char*)foo.Get();
> *lpszCanChange = NULL; // BREAKS DATA HIDING
> }
>
> You see the problem. Even though m_szText is private, it is still
> possible to alter its value and so the state of CFoo's instance. You
> cannot do
> strncpy(foo.m_szText, "New Text", sizeof(foo.m_szText)) but above
> 'loophole' would break object-orientedness.
C++'s access mechanisms were designed to protect against accident,
*not* against fraud.
>
> Only way to prevent this is to make a copy of it and return it, as far
> as I know. Does somebody know how to resolve this without copying it?
If you have fellow programmers who cast away constness causually, you
have a problem no feature of any programing language can hope to
solve.
Fortunately, a little education may solve the problem.
> You see the problem. Even though m_szText is private, it is still
> possible to alter its value and so the state of CFoo's instance. You
> cannot do strncpy(foo.m_szText, "New Text", sizeof(foo.m_szText)) but above
> 'loophole' would break object-orientedness.
Your program has full access to its own process's address space. In
that regard, you cannot make data truly hidden from other parts of
your program. You can make it obscure or difficult to reach, but
there is always a way.
The example above shows malace on the user who is bypassing your
constness. Casting away const could result in undefined behavior as
well.
> Only way to prevent this is to make a copy of it and return it, as far
> as I know. Does somebody know how to resolve this without copying it?
The data must exist somewhere. If you give them direct read-access to
the data, it doesn't matter what you do, they can untie any knot you
try to tie. You can use extra indirection, make them chase pointers,
etc, but at the end of the day, your memory is still accessible for
them to scribble, if only they know where to find it.
As they say, you can protect against Murphy, but not Machiavelli.
Afterall, nothing prevents someone from casting your object to a void*
and scanning the bits. Again, though it's undefined, someone with
malice probably doesn't care.
Chris
> ps wrote:
>
> > Even though m_szText is private, it is still possible to alter its
> > value and so the state of CFoo's instance.
[reinserting some elided text:]
> > above 'loophole' would break object-orientedness.
> Yes, and that can be done by writing "*(char *)&foo = 0;" as well.
> There's nothing you can do to prevent wilful violation of
> encapsulation in another part of the program.
#define private public
... all of which conclusively proves that C++ is not an Object
Oriented language ... right ?
Or maybe we should re-examine this daft idea that encapsulation is
synonymous to access restriction, and that access restriction is
somehow a vital prerequisite for object oriented
programming. Encapsulation amounts to providing an interface to your
objects; data absttraction, essentially. Access restriction is a
completely orthogonal issue.
Object oriented programming is merely a style of programming; yes, you
can even do it in C. It just so happens that some languages try to
help you in writing in that style, by providing built-in support for
things such as encapsulation, polymorphism and inheritance. Some of
those languages augment encapsulation with access control; they
attempt to make it impossible to look inside the capsule. I consider
it a great shame that various C++ and Java schools of thought have
polluted the minds of thousands of innocent souls making them believe
to believe that accsess restriction is fundamental to object oriented
programming.
Questions to the OP:
Consider the following languages: Smalltalk, Common Lisp, Python
(to name but a few)
For each of them, do you think that
a) It is object oriented ?
b) It has access restriction ?
In summary: I think that you are wasting your time with this. You'd be
so much more productive if you just got on with implementing
functionality.
What is LPCTSTR? The Standard defines no such type. Don't assume
that just because your compiler supplies additional bizarrely named
typedefs that everyone will be familiar with them. For the sake of
argument, let's assume
typedef char const* LPCTSTR;
> LPCTSTR Get() const { return m_szText; }
>
> private:
> char m_szText[256];
> };
>
> int main()
> {
> CFoo foo("My text");
> LPCTSTR lpszCantChange = foo.Get();
> *lpszCantChange = NULL; // COMPILATION ERROR. GOOD.
>
> char* lpszCanChange = (char*)foo.Get();
> *lpszCanChange = NULL; // BREAKS DATA HIDING
> }
>
> You see the problem.
Yes. The problem is that the user of the class is breaking its
contract with the class.
> Even though m_szText is private, it is still
> possible to alter its value and so the state of CFoo's instance. You
> cannot do
> strncpy(foo.m_szText, "New Text", sizeof(foo.m_szText)) but above
> 'loophole' would break object-orientedness.
To get at this so-called loophole, you are using a const_cast
(implicitly via the C-style cast) to circumvent C++'s type-system. In
many situations this actually causes undefined behaviour, although
this is not such a case. (I think the following would be an example
of undefined behaviour:
int main() {
CFoo const foo( "fish soup" );
const_cast< char* >( foo.Get() )[0] = 'F';
}
which causes undefined behaviour because the compiler might have been
able to put the foo.m_szText into read-only memory.)
> Only way to prevent this is to make a copy of it and return it, as far
> as I know.
This certainly would prevent it, but if you expect users of the class
to be circumventing the typesystem in this manner, how do you know
they won't be doing it in other ways? For instance
class CFoo2 {
friend int main();
char m_szText[256];
};
int main() {
CFoo foo( "fish soup" );
reinterpret_cast< CFoo2& >( foo ).m_szText[0] = 'F';
}
(Of course, there's no guarantee that this will work, but I'd be
surprised if it didn't.)
> Does somebody know how to resolve this without copying it?
Always avoid C-style casts and avoid const_casts unless you are sure
you need them. If you have legacy interfaces that take non-const
objects despite never writing them, provide wrappers to hide this. If
you stick to a few simple rules like this, your code will be a lot
cleaner and won't suffer from this sort of problem. C++'s
const-correctness is there to help you, and if you don't abuse it, it
will.
--
Richard Smith
LPCTSTR isn't standard C++, but I'm assuming it's a VC++ macro for
const char*. Given this assumption, I have two points to make:
> class CFoo
> {
> public:
> /* ... */
> LPCTSTR Get() const { return m_szText; }
> /* ... */
>
> private:
> char m_szText[256];
> };
>
> int main() {
>
> char* lpszCanChange = (char*)foo.Get();
> }
1. Get() returns a const pointer to the data member, and so does not
in itself break encapsulation; the C-style cast in the assignment
above is the culprit. The language provides some safeguards (i.e. not
being able to return non-const pointers or references to class members
from const-qualified member functions), but you can always subvert
them with careless casting. Cast with (extreme) caution.
> *lpszCanChange = NULL; // BREAKS DATA HIDING
2. This assignment is pretty strange. NULL should only ever be
assigned to a pointer value; here, you're trying to assign the NULL
value to a char (namely, foo.m_szText[0]). The compiler will probably
allow this, by converting NULL to (char)0, but is this what you really
want?
I think the point you were trying to make was that e.g.
strcpy(lpszCanChange, "Whatever");
would compile; again, that's down to the effect of the cast above.
Gareth
Use C++, not C with Classes.
using std::string;
class Foo
{
public:
Foo( string s) : m_s(s) { }
string const& Get() const { return m_s; }
void Set(string const& s) { m_s = s ; }
private:
string m_s;
};
string const& CantChange = Foo("X").Get(); // Can't change, no copy
string CanChange = Foo("X").Get(); // Can change, because it's a copy
string& TryToBreak = Foo("X").Get(); // Caught by compiler
Regards,
Michiel Salters
> Only way to prevent this is to make a copy of it and return it, as far
> as I know. Does somebody know how to resolve this without copying it?
No. If you want the data to be completely hidden, perhaps you
shouldn't provide a get operation - just provide operations that allow
comparison with other char *s, etc. Alternatively, you could use the
std::string class. Even if you do that, there is nothing to stop
someone writing
class Opaque
{
friend Exciting_Class;
public:
//The only thing you can do with this class if you aren't
//exciting class is copy it, compare it, assign it or have
//an instance of it.
/* define default constructor, assignment & equality operators
*/
private:
int data
};
The above class is entirely opaque except to the Exciting_Class. So
you should only be able to do:
Exciting_Class fred, bert;
Opaque o1, o2;
o1 = fred.get_opaque_value();
o2 = bert.get_opaque_value();
if (o1 == o2) { ... }
You can't assign values to o1, you can't extract the value from o1,
all you can do is compare it with another Opaque value. However, that
doesn't stop the following code:
int i;
Opaque j;
//...
i = *(int *)&j;
*(int *)j = 0;