Passing a pointer of constructor

[seank76]

While going through my company's existing codebase, I saw a bunch of
weird lines.

Take a look at this one:

class A
{
public:
A(Foo *f) : _f(f) {}

private:
Foo *_f;
};

class B : public A
{
public:
B() : A(&(Foo &)Foo())) {}
};

How can you pass "Foo()" as the argument to the constructor of A ?
Wouldn't the memory that is allocated to Foo() get destroyed as soon
as we are out of the scope of B()?

What does this mean?

[seank76]

On May 4, 4:56 am, James Kanze <james.ka...@gm ail.comwrote:

On May 3, 10:23 pm, seank76 <sean...@gmail. comwrote:

On May 3, 4:09 pm, "Alf P. Steinbach" <[e-mail] rote:

* seank76:
On May 3, 2:51 pm, Gianni Mariani <gi3nos...@mari ani.wswrote:
seank76 wrote:
>While going through my company's existing codebase, I saw a bunchof
>weird lines.
>Take a look at this one:
>class A
>{
> public:
> A(Foo *f) : _f(f) {}
> private:
> Foo *_f;
>};
>class B : public A
>{
> public:
> B() : A(&(Foo &)Foo())) {}
>};
That was my first response when I first saw this code.
However, a colleague of mine has claimed although it is "temporary" ,
the scope of it belongs to the class instance, NOT the constructor,
hence it is ok.
Bullshit.

Just compiled and ran a test program without any errors using Sun
forte and gcc1.2

Is that g++ 1.2? g++ is currently at 4.1.something, or more.
Any g++ before about 2.6 or 2.7 is totally worthless. Any g++
before 3.0 is hopelessly out of date.

the test program is the following:
#include <iostream>
#include <fstream>
using namespace std;
class A
{
public:
A() {}
void doThat() { cout << "HELLOSE SEAN!" << endl; }

};
class B
{
protected:
A* _a;
public:
B(A *a) : _a(a) {}
};
class C : B
{
public:
C() : B((A *)&(const A &)A()) {}

You added a const. That makes the code syntactically correct,
and means that the compiler is not required to issue a
diagnostic.

void dodo() { _a->doThat(); }
};
int main()
{
cout << "Starting test." << endl;
C c;
cout << "About the run C.dodo() " << endl;
c.dodo();
return 0;
}

The code contains undefined behavior, which means that it might
seem to work. Especially in simple cases. Try adding tracing
calls to the constructors (including the copy constructor, just
in case) and the destructor of A:

class A
{
public:
A() { std::cout << "A::ctor" << std::endl ; }
A( A const& ) { std::cout << "A::copy" << std::endl ; }
~A() { std::cout << "A::dtor" << std::endl ; }
// ...
} ;

With an up-to-date, conformant compiler, you should see the
message from the destructor before the message from A::doThat().

Another thing you might try is to add a member variable to A,
initialize it in the constructor, overwrite it in the
destructor, and output its value in A::doThat(); you should find
that it does not have the correct value. (The reason your code
seems to work, of course, is because you don't actually use
anything in A in doThat(). Just making doThat() virtual might
be enough to cause problems.)

--
James Kanze (GABI Software) email:james.ka. [e-mail]
Conseils en informatique orientée objet/
Beratung in objektorientier ter Datenverarbeitu ng
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

Yeah... the problem with big companies is that the codebase is usually
10+ years old and compilers they use are horribly out-of-date.
I'm trying to change that by upgrading things here and there.
However, as you all probably know, the number one rule in business is,
don't fix it if it ain't broke.
So I need to show convincing reasonings before changing a line in the
code that's been in use for the last 7 or so years.

Now that I understand exactly what's going on, (thanks to James and
Olf Wolf) I will now attempt to fix the code.

thanks,
Sean.

[seank76]

On May 4, 5:04 am, James Kanze <james.ka...@gm ail.comwrote:

On May 3, 11:58 pm, seank76 <sean...@gmail. comwrote:

On May 3, 5:55 pm, "Alf P. Steinbach" <[e-mail] rote:

* seank76:
[quoting signatures]
Please don't quote signatures, please read the FAQ before posting.

Please don't say Bullshit to a topic you don't understand.

What your collegue claimed about the program was bullshit. It
shows simply that he doesn't really know C++.

You asked about particular code. We're telling you. It's not
legal. It never has been. And although it may work with some
older compilers, it would never pass code review in any well run
shop.

If it works, today, it is purely by chance, because you have a
compiler which is out of date, or because the undefined behavior
doesn't happen to reveal the error.

Please read "Internet Etiquette" manual before you post.

He did, I think. At least, it does say to not quote signatures.

I think either I was posting in a different language or some of you
don't read the postings carefully enough.
You guys make it seem as though I was arguing the code is legal and
you all should go to hell or something.

I thought the code looked weird and probably not right.
But I know it compiles and works right now. (whether it's C++ standard
or platform agnostic is irrelevant in a production environment unless
you are a software shop.)
So I was asking why and how it works.

Now, if you tell me "it's illegal" or "go buy a lottery ticket", I
just wasted my time posting to a newsgroup.
If you didn't understand what I was saying, or I really was talking
out of my ass, at least have the courtesy to just skip to the next
message.

Oh one more thing about what my "colleague" said.
As I know how computer programmers think, we tend to think nobody else
but me truly understands how to program since program is an art, not a
science. (...right?)
Therefore when somebody brings up a point that seems totally against
my belief, we tend to turn our heads and say "bullshit".
However, if you actually put in the time to think for a second instead
of typing away those 8 letters, you would have realized the
initialization block of the constructor is actually outside of the
bracketed scope and really belongs to the class instance scope
(basically the same scope as the definition of the constructor
itself).
Now, I didn't know C++ standard specifically limited the compilers to
destroy any temporaries after the initialization is done.
This might be true and sub-skilled programmers like myself come to a
forum like this one to seek such valuable knowledges.

If this was such a wide-known fact that a 4-year old should have
known, I sincerely apologies for my lack of intelligence.

thanks,
Sean

[Victor Bazarov]

seank76 wrote:

[..]
So I was asking why and how it works.

"Works" is the wrong term. "Appears to work" is better.

And who the hell knows? The behaviour of the code is undefined
and there is nothing anybody can tell you except that it "works"
by pure luck. Going into speculations on how electrical signals
on the motherboard of your computer make it so happen that it
seems to "work" for you is not what we do here in comp.lang.c++.
That would really be a waste of time. Honestly. It has no merit.

Now, if you tell me "it's illegal" or "go buy a lottery ticket", I
just wasted my time posting to a newsgroup.

Yes, you did. Once somebody tells you "the behaviour of such and
such code is undefined", you can stop asking "why it works" right
there and then. Write that on a piece of paper and hang it on the
wall where it will catch your eye every so often.

If you didn't understand what I was saying, or I really was talking
out of my ass, at least have the courtesy to just skip to the next
message.

If your post would have been left alone, how would you then know
whether in fact you were talking nonsense or nobody simply cared
to read or understand what you were saying? At least here you've
got to the bottom of it (or so it seems).

[..]

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[Lionel B]

On Fri, 04 May 2007 07:02:06 -0700, seank76 wrote:

On May 4, 5:04 am, James Kanze <james.ka...@gm ail.comwrote:

>On May 3, 11:58 pm, seank76 <sean...@gmail. comwrote:

On May 3, 5:55 pm, "Alf P. Steinbach" <[e-mail] rote:

[...]

I think either I was posting in a different language or some of you
don't read the postings carefully enough. You guys make it seem as
though I was arguing the code is legal and you all should go to hell or
something.

[more ranting snipped]

Hey, relax; you posted a reasonable question and got several, correct
answers (in a nutshell: your code has UB, your colleague was wrong) along
with detailed explanations, from people who know what they are talking
about; ok, some of the responses didn't mince words, but that's usenet
for you.

You got the best result you could hope for - be happy.

--
Lionel B

[seank76]

On May 4, 10:19 am, "Victor Bazarov" <v.Abaza...@com Acast.netwrote:

seank76 wrote:

[..]
So I was asking why and how it works.

"Works" is the wrong term. "Appears to work" is better.

And who the hell knows? The behaviour of the code is undefined
and there is nothing anybody can tell you except that it "works"
by pure luck. Going into speculations on how electrical signals
on the motherboard of your computer make it so happen that it
seems to "work" for you is not what we do here in comp.lang.c++.
That would really be a waste of time. Honestly. It has no merit.

Now, if you tell me "it's illegal" or "go buy a lottery ticket", I
just wasted my time posting to a newsgroup.

Yes, you did. Once somebody tells you "the behaviour of such and
such code is undefined", you can stop asking "why it works" right
there and then. Write that on a piece of paper and hang it on the
wall where it will catch your eye every so often.

If you didn't understand what I was saying, or I really was talking
out of my ass, at least have the courtesy to just skip to the next
message.

If your post would have been left alone, how would you then know
whether in fact you were talking nonsense or nobody simply cared
to read or understand what you were saying? At least here you've
got to the bottom of it (or so it seems).

Victor,

If you have told me that "the behavior of such and such code is
undefined since it violates such and such rule", there is no more
discussion as you have so correctly put it.

It so happens that wasn't the response I got from you or anyone at the
point of time. (In fact I'm not even sure if you or Alf gave me any
responses at all)
As you can see, when some people who know what they are talking about
replied with real answers, I was one happy camper.

So we should add it to the "FAQ" to only respond with real answers to
the questions.
I think this might be a bit more important that removing the
signature.

thanks,
Sean

[Victor Bazarov]

seank76 wrote:

[..]
If you have told me that "the behavior of such and such code is
undefined since it violates such and such rule", there is no more
discussion as you have so correctly put it.

Yes, in _my_ response the "such and such rule" was missing. But in
his first post to this thread Gianni Mariani told you that you were
initialising something with an address of a temporary that was not
going to survive beyond the constructor's execution. Then you told
us that your colleague claimed that it's OK. Alf told you that it
was BS. It was. Your colleague was BSing you. Most likely from his
or her ignorance.

It so happens that wasn't the response I got from you or anyone at the
point of time. (In fact I'm not even sure if you or Alf gave me any
responses at all)

Too bad. You seem to have been too wrapped up in trying to defend
your colleague's point of view or getting offended by the flames you
received. <shrug Next time wear your flame-retardant shorts.

As you can see, when some people who know what they are talking about
replied with real answers, I was one happy camper.

<shrugNot everybody is in the business of making campers happy.

So we should add it to the "FAQ" to only respond with real answers to
the questions.

The FAQ is not the law, it cannot be enforced in an unmoderated NG.
If you need controlled environment, try 'comp.lang.c++. moderated'.
Here I, or anybody else, respond as they deem fit. Killfile me if
you don't like my responses. I won't feel it, don't you worry.

I think this might be a bit more important that removing the
signature.

It might.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[Gianni Mariani]

seank76 wrote:
....

>I stress that this is NOT defined behaviour and is just a quirk
of your system. It might suddenly stop working at any moment.

Thanks for the answer.
This is what I was looking for, not some little kids telling me it's
bad code.
I am the one who is telling people around me it's bad code.
I just need to convince them why it's bad.

Sean,

a) Everyone who has responded to you has been very courteous and
genuinely trying to help *you*.

b) If you don't understand why something is *bad code*, don't shoot the
messenger. Try to be a little introspective. (Who is the baby here?)

c) Usenet/emails are very often misread (as in the tone of the
document). Get beyond the perceived tone of usenet postings, in 99% of
cases, the tone you interpret is not the tone that was intended. In
other words, if someone seems to be extra nice, they're just as likely
extra annoyed.

d) You got your answer from 4 very experienced posters. I know they are
very experienced because I see their answers on this NG all the time and
the only errors I see they make are typos. Be happy.

e) Given the level of your question, this is not likely the only
question you're going to have. Annoying 4 of the regular posters with
assertions of being "children" is not likely to solicit any help next
time you need some.

Good luck.

Tell your boss I charge at $200/hr and you got 20 minutes of my time for
free. If he does not believe me that the code you have shown is a
problem, then I'd be more that willing to consult to confirm or deny.
Minimum 8hrs (not free).

[pmouse]

On May 4, 11:51 am, seank76 <sean...@gmail. comwrote:

On May 4, 10:19 am, "Victor Bazarov" <v.Abaza...@com Acast.netwrote:

seank76 wrote:

[..]
So I was asking why and how it works.

"Works" is the wrong term. "Appears to work" is better.

And who the hell knows? The behaviour of the code is undefined
and there is nothing anybody can tell you except that it "works"
by pure luck. Going into speculations on how electrical signals
on the motherboard of your computer make it so happen that it
seems to "work" for you is not what we do here in comp.lang.c++.
That would really be a waste of time. Honestly. It has no merit.

Now, if you tell me "it's illegal" or "go buy a lottery ticket", I
just wasted my time posting to a newsgroup.

Yes, you did. Once somebody tells you "the behaviour of such and
such code is undefined", you can stop asking "why it works" right
there and then. Write that on a piece of paper and hang it on the
wall where it will catch your eye every so often.

If you didn't understand what I was saying, or I really was talking
out of my ass, at least have the courtesy to just skip to the next
message.

If your post would have been left alone, how would you then know
whether in fact you were talking nonsense or nobody simply cared
to read or understand what you were saying? At least here you've
got to the bottom of it (or so it seems).

Victor,

If you have told me that "the behavior of such and such code is
undefined since it violates such and such rule", there is no more
discussion as you have so correctly put it.

It so happens that wasn't the response I got from you or anyone at the
point of time. (In fact I'm not even sure if you or Alf gave me any
responses at all)
As you can see, when some people who know what they are talking about
replied with real answers, I was one happy camper.

So we should add it to the "FAQ" to only respond with real answers to
the questions.
I think this might be a bit more important that removing the
signature.

thanks,
Sean

I thought my reply in the first post tells u exactly what rule this
code violates, the g++ message is clear enough: you cannot cast a
rvalue returned by Foo() to Foo&

that is, the constructor Foo() returns a const reference type of Foo,
you simply cannot cast that to a lvalue and hope for the code to still
make sense - if your compiler compiles the code shown in your post,
then it needs to be thrown away.

Thing will be different, of course, if you are willing to use a "const
Foo&" instead. Modify A&B to look like:
class A
{
public:
A( const Foo* pf ) : _pF(pf) {}
const Foo* _pF;
};

class B : public A
{
public:
B() : A( &(const Foo&)Foo() ){}
};
will allow the code to compile.

What exactly is B() doing then?
Well, let's see, it creates a Foo object on the stack and get the
address of that object, then store it inside a constant pointer. I
think you and I both agree to this point. The question is then when
will the new Foo() object get destroyed? My bet it since it sits on
the stack, as soon as the function returns, it will get destroyed.
Your coworkers who wrote this code claims that the object has a
"scope" within the class, therefore it gets destroyed with the class -
I'm afraid that this claim makes no sense at all.

A simple test reveals what happens. Let's modify Foo to look like
this:
class Foo
{
public:
Foo() :a (0){ cout << "Foo()" << endl; }
~Foo() { cout << "~Foo()" << endl; }
Foo( const Foo& f ) : a(f.a) { cout << "copy Foo()" << endl; }
int a;
};

and let's declare a object of type B, then access the constant pointer
inside B to print out Foo::a:
B b;
cout << b._pF->a << endl;

Let's compile and run it:

peter@peter-laptop ~ $ g++ test.cpp -o test
peter@peter-laptop ~ $ ./test
Foo()
~Foo()
0

As demonstrated, The class's destructor is called before the access -
you are accessing memory that is not allocated, although the value did
print out correctly, that is only because no other object currently
uses that space yet. But this is now a potential bomb and will cost
your company a fortune to debug once it explodes.

What your coworker probably meant to do is:
B : A( new Foo() ) {}

and

~A () { if ( _pF ) delete _pF; }

if there exist a syntactically and logically correct answer, why not
stick to it?

Regards,

PQ

[Gianni Mariani]

pmouse wrote:

On May 4, 11:51 am, seank76 <sean...@gmail. comwrote:

....

~A () { if ( _pF ) delete _pF; }

~A () { delete _pF; }

The if ( _pF ) is already checked by delete.

[pmouse]

On May 4, 10:14 pm, Gianni Mariani <gi3nos...@mari ani.wswrote:

pmouse wrote:

On May 4, 11:51 am, seank76 <sean...@gmail. comwrote:

...

~A () { if ( _pF ) delete _pF; }

~A () { delete _pF; }

The if ( _pF ) is already checked by delete.

Not in GCC/g++, and many other compilers that I use.
deleting a piece of memory twice will cause a double free exception
(run time)
On the other hand, free() does check for zero pointers.

Regards,

PQ