Overview
Examples
Screenshots
Comparisons
Applications
Download
Documentation
Tutorials
Bazaar
Status & Roadmap
FAQ
Authors & License
Forums
Funding Ultimate++
Search on this site
Search in forums












SourceForge.net Logo
Home » U++ TheIDE » U++ TheIDE: Other Features Wishlist and/or Bugs » bug in latest svn
Re: bug in latest svn [message #15673 is a reply to message #15668] Sun, 04 May 2008 03:35 Go to previous messageGo to next message
Novo is currently offline  Novo
Messages: 1358
Registered: December 2006
Ultimate Contributor
mdelfede wrote on Sat, 03 May 2008 13:17


AFAIK that's a right behavior, as returned temporary string is firs assigned to result and then deleted.



IMHO, the way you describe would work if the function looks like below.

String GetCppFile(int i);



Current implementation is similar to code below.

const String* GetCppFile(int i)
{
	INTERLOCKED_(cpp_file_mutex) {
		return &cpp_file[i];
	}
	return &String();
}


Does this look correct to you?


Regards,
Novo

[Updated on: Sun, 04 May 2008 03:43]

Report message to a moderator

Re: bug in latest svn [message #15679 is a reply to message #15673] Sun, 04 May 2008 12:17 Go to previous messageGo to next message
mdelfede is currently offline  mdelfede
Messages: 1307
Registered: September 2007
Ultimate Contributor
Novo wrote on Sun, 04 May 2008 03:35

mdelfede wrote on Sat, 03 May 2008 13:17


AFAIK that's a right behavior, as returned temporary string is firs assigned to result and then deleted.



IMHO, the way you describe would work if the function looks like below.

String GetCppFile(int i);



Current implementation is similar to code below.

const String* GetCppFile(int i)
{
	INTERLOCKED_(cpp_file_mutex) {
		return &cpp_file[i];
	}
	return &String();
}


Does this look correct to you?



Nope, I guess... here you're returning a reference to a local variable, that can (and usually IS) destroyed BEFORE function returns. So, you could make it static to solve the problem, but then you'd have many other problems, in particular with MT.
Returning a String() value is less efficient, but guarantees that string is not destroyed on function return before the value is taken.

Max

Re: bug in latest svn [message #15681 is a reply to message #15679] Sun, 04 May 2008 17:04 Go to previous messageGo to next message
Novo is currently offline  Novo
Messages: 1358
Registered: December 2006
Ultimate Contributor
mdelfede wrote on Sun, 04 May 2008 06:17

Returning a String() value is less efficient, but guarantees that string is not destroyed on function return before the value is taken.



Isn't

return String();


equal to

String anonymous_value;
return anonymous_value;


?

const String& GetCppFile(int i);

String value1 = GetCppFile(0); // Case A
const String& value2 = GetCppFile(0); // Case B


In case A GetCppFile() will work correctly.
Case B will introduce a bug.

IMHO, returning "const String&" is just not thread-safe. Object can be deleted in transition.


Regards,
Novo
Re: bug in latest svn [message #15682 is a reply to message #15679] Sun, 04 May 2008 17:11 Go to previous messageGo to next message
Novo is currently offline  Novo
Messages: 1358
Registered: December 2006
Ultimate Contributor
mdelfede wrote on Sun, 04 May 2008 06:17

Nope, I guess... here you're returning a reference to a local variable, that can (and usually IS) destroyed BEFORE function returns.



IMHO, returning reference to a local variable is the same thing as returning pointer to a local variable ...


Regards,
Novo
Re: bug in latest svn [message #15683 is a reply to message #15681] Sun, 04 May 2008 17:14 Go to previous messageGo to next message
mdelfede is currently offline  mdelfede
Messages: 1307
Registered: September 2007
Ultimate Contributor
Novo wrote on Sun, 04 May 2008 17:04


const String& GetCppFile(int i);

String value1 = GetCppFile(0); // Case A
const String& value2 = GetCppFile(0); // Case B


In case A GetCppFile() will work correctly.
Case B will introduce a bug.

IMHO, returning "const String&" is just not thread-safe. Object can be deleted in transition.



I guess you're right, with your 2 examples.
On first example it 'should' work, because the source string isn't deleted before assignment, so the value is transferred. The secon example is buggy because you return a reference to an object that'll be deleted soon. BTW, I don't know how does the compiler behave with the const modifier.... but I guess it'll not solve the problem.

So the code is bad, imo.... is that maybe the problem ?

Max

Re: bug in latest svn [message #15684 is a reply to message #15682] Sun, 04 May 2008 17:16 Go to previous messageGo to next message
mdelfede is currently offline  mdelfede
Messages: 1307
Registered: September 2007
Ultimate Contributor
Novo wrote on Sun, 04 May 2008 17:11

mdelfede wrote on Sun, 04 May 2008 06:17

Nope, I guess... here you're returning a reference to a local variable, that can (and usually IS) destroyed BEFORE function returns.



IMHO, returning reference to a local variable is the same thing as returning pointer to a local variable ...



Yes, it is, and you're right, it's a dangerous way.
It's strange that compiler doesn't warn about....

Max
Re: bug in latest svn [message #15685 is a reply to message #15684] Sun, 04 May 2008 17:43 Go to previous messageGo to next message
mdelfede is currently offline  mdelfede
Messages: 1307
Registered: September 2007
Ultimate Contributor
This one shows that you're right...

long retlong(void)
{
	static long l = 5;
	return l++;
	
}

const long &test(void)
{
	return retlong();
	
}

int main(int argc, const char *argv[])
{
	const long &a = test();
	const long &b = test();
	
	long aa = a;
	long bb = b;
	
	return 0;
}



In my test run, both aa and bb get value of 6.

Max
Re: bug in latest svn [message #15686 is a reply to message #15684] Sun, 04 May 2008 18:07 Go to previous messageGo to next message
Novo is currently offline  Novo
Messages: 1358
Registered: December 2006
Ultimate Contributor
mdelfede wrote on Sun, 04 May 2008 11:16


It's strange that compiler doesn't warn about....



Actually, it does ...



Regards,
Novo
Re: bug in latest svn [message #15687 is a reply to message #15686] Sun, 04 May 2008 18:12 Go to previous messageGo to next message
mdelfede is currently offline  mdelfede
Messages: 1307
Registered: September 2007
Ultimate Contributor
Indeed, I was wrong before :

This piece of code :

#include "stdio.h"

static long seed = 0;

class LongClass
{
		long l;
	public :
		long Get(void) const { return l; }
		LongClass() { l = seed++ ; }
	
};

const LongClass &test(const LongClass &lClass = LongClass())
{
	return lClass;
	
}

int main(int argc, const char *argv[])
{
	const LongClass &a = test();
	const LongClass &b = test();
	
	long aa = a.Get();
	long bb = b.Get();
	
	return 0;
}


Shows that's perfectly legal to return references to const temporaries... Here aa gets a value of 0 and bb of 1, correctly.

That behaviour allows to initialize default reference arguments with objects created on the fly. I don't know where does compile store the actual value....

Max

[Updated on: Sun, 04 May 2008 18:14]

Report message to a moderator

Re: bug in latest svn [message #15689 is a reply to message #15667] Sun, 04 May 2008 18:28 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 13975
Registered: November 2005
Ultimate Member
Novo wrote on Sat, 03 May 2008 11:42

Novo wrote on Fri, 02 May 2008 16:15


Hopefully, I’ll submit a patch at the end of the day.



Sorry, couldn't keep my promice.

I probably miss something, but code below always was a problem.

const String& GetCppFile(int i)
{
	INTERLOCKED_(cpp_file_mutex) {
		return cpp_file[i];
	}
	return String();
}


Should be something like that:

const String& GetCppFile(int i)
{
	INTERLOCKED_(cpp_file_mutex) {
		return cpp_file[i];
	}

        static String value;
	return value;
}



I'm not sure about thread-safety of static variable initialization though.



Well, second return is never performed. It is there only to shut-up the compiler about missing return.... (which, of course, in this context sounds like void effort Smile

Mirek

P.S.: Other than that, while working in the current IDE, the code should be changed anyway.
Re: bug in latest svn [message #15696 is a reply to message #15689] Sun, 04 May 2008 21:46 Go to previous messageGo to next message
mdelfede is currently offline  mdelfede
Messages: 1307
Registered: September 2007
Ultimate Contributor
Well... maybe found bug's location.
It's on String.cpp, Alloc() or something near this.

Investigating a bit more....

Max
Re: bug in latest svn [message #15721 is a reply to message #15689] Tue, 06 May 2008 04:07 Go to previous messageGo to next message
Novo is currently offline  Novo
Messages: 1358
Registered: December 2006
Ultimate Contributor
luzr wrote on Sun, 04 May 2008 12:28

Well, second return is never performed. It is there only to shut-up the compiler about missing return.... (which, of course, in this context sounds like void effort Smile



In this case I have two questions:

1) Why you are not using something like mutex guard for such code? That would eliminate requirement in "return String()".

2) Why this code was left not thread-safe?

Just curios ...


Regards,
Novo
Re: bug in latest svn [message #15722 is a reply to message #15687] Tue, 06 May 2008 04:10 Go to previous messageGo to next message
Novo is currently offline  Novo
Messages: 1358
Registered: December 2006
Ultimate Contributor
mdelfede wrote on Sun, 04 May 2008 12:12


Shows that's perfectly legal to return references to const temporaries... Here aa gets a value of 0 and bb of 1, correctly.



Believe me, that is not safe Wink


Regards,
Novo
Re: bug in latest svn [message #15763 is a reply to message #15721] Wed, 07 May 2008 13:59 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 13975
Registered: November 2005
Ultimate Member
Novo wrote on Mon, 05 May 2008 22:07

luzr wrote on Sun, 04 May 2008 12:28

Well, second return is never performed. It is there only to shut-up the compiler about missing return.... (which, of course, in this context sounds like void effort Smile



In this case I have two questions:

1) Why you are not using something like mutex guard for such code? That would eliminate requirement in "return String()".

2) Why this code was left not thread-safe?

Just curios ...



Well, INTERLOCKED_ is a mutex guard...

Unfortunately, the compiler is not able to figure out that second return never takes place...

Mirek
Re: bug in latest svn [message #15765 is a reply to message #15618] Wed, 07 May 2008 14:13 Go to previous messageGo to next message
mr_ped is currently offline  mr_ped
Messages: 825
Registered: November 2005
Location: Czech Republic - Praha
Experienced Contributor
The warning may be avoided trough:
static const String interlock_problem = "Some problem with INTERLOCKED_ happened!";
const String& GetCppFile(int i)
{
	INTERLOCKED_(cpp_file_mutex) {
		return cpp_file[i];
	}
	return interlock_problem;
}



This will also lead to weird String constant in case the INTERLOCKED_ macro gets damaged by something and it will stop work correctly.
(and returning the interlock_problem const reference is thread safe no matter what? I wish I would KNOW, I will have to work on my "knowledge" of ordinary C++ MT and U++ way. The general ASM knowledge doesn't help here too much.)
Re: bug in latest svn [message #15767 is a reply to message #15763] Wed, 07 May 2008 14:39 Go to previous messageGo to next message
mdelfede is currently offline  mdelfede
Messages: 1307
Registered: September 2007
Ultimate Contributor
luzr wrote on Wed, 07 May 2008 13:59

Novo wrote on Mon, 05 May 2008 22:07

luzr wrote on Sun, 04 May 2008 12:28

Well, second return is never performed. It is there only to shut-up the compiler about missing return.... (which, of course, in this context sounds like void effort Smile



In this case I have two questions:

1) Why you are not using something like mutex guard for such code? That would eliminate requirement in "return String()".

2) Why this code was left not thread-safe?

Just curios ...



Well, INTERLOCKED_ is a mutex guard...

Unfortunately, the compiler is not able to figure out that second return never takes place...

Mirek



It should be enough to place a #pragma warn somewhere to disable warning.

Max
Re: bug in latest svn [message #15781 is a reply to message #15763] Wed, 07 May 2008 22:07 Go to previous message
Novo is currently offline  Novo
Messages: 1358
Registered: December 2006
Ultimate Contributor
luzr wrote on Wed, 07 May 2008 07:59

Novo wrote on Mon, 05 May 2008 22:07

luzr wrote on Sun, 04 May 2008 12:28

Well, second return is never performed. It is there only to shut-up the compiler about missing return.... (which, of course, in this context sounds like void effort Smile



In this case I have two questions:

1) Why you are not using something like mutex guard for such code? That would eliminate requirement in "return String()".

2) Why this code was left not thread-safe?

Just curios ...



Well, INTERLOCKED_ is a mutex guard...

Unfortunately, the compiler is not able to figure out that second return never takes place...

Mirek



What is wrong with the code below?

const String& GetCppFile(int i)
{
	Mutex::Lock guard(cpp_file_mutex);

	return cpp_file[i];
}


What is so special about INTERLOCKED_ ?


Regards,
Novo

[Updated on: Wed, 07 May 2008 22:07]

Report message to a moderator

Previous Topic: Some lack of features
Next Topic: [BUG] Ide loses "maximized" setting on minimize
Goto Forum:
  


Current Time: Tue Apr 16 15:38:06 CEST 2024

Total time taken to generate the page: 0.02165 seconds