Disturbing: const QString &

**magland** · 15th November 2007, 19:55

I recently found a bug in one of my programs... and it's disturbing to me, so I'd like some comments.

Suppose I have a function which takes QString as an argument, and I don't change the value of the argument in the function, so I set it as const (is that what const means by the way?... that the function is not allowed to change the value of it?).

But the function happened to be recursive, and it called itself with another value of the parameter. This caused UNEXPECTED BEHAVIOR! I can't describe exactly what happens, but the values changed right in the middle of the function!

Qt Code:

Switch view

void myfunction(const QString &val) {
  ...
  QString val2=...;
  ...
  myfunction(val2);
  [Here's where val changed!]
  ...
}

void myfunction(const QString &val) {
  ...
  QString val2=...;
  ...
  myfunction(val2);
  [Here's where val changed!]
  ...
}

To copy to clipboard, switch view to plain text mode

Am I misunderstanding something?

**marcel** · 15th November 2007, 20:36

Are you sure?
Can you post the entire function?

**magland** · 16th November 2007, 02:58

Here's the function. It's part of a big program, and I can't isolate the problem easily. But the fact is that when I take out the "QString path=path_in;" line and replace "path_in" by "path" in the function arguments, I get a serious problem.

I think this could be a bug with MinGW g++ ? !!!

Qt Code:

Switch view

void update_node(const QString &path_in,bool do_emit_signal) {
		QString path=path_in; //apparently this is important :(
		if (!nodes.contains(path)) return;
		if (!nodes[path].needs_to_be_updated) return;
		nodes[path].needs_to_be_updated=false;
		nodes[path].children.clear();
		QList<HaiQTag> tags=core->tagManager->byPath(path);
		QStringList search_paths=include_search_paths;
		search_paths << QFileInfo(path).path();
		QStringList included_paths;
		for (int j=0; j<tags.count(); j++) {
			if (tags[j].tagtype=="pound_include") {
				QString holdstr=get_include_file_path(tags[j].signature,search_paths);;
				if (!holdstr.isEmpty())
					included_paths << holdstr;
			}
		}
		for (int j=0; j<included_paths.count(); j++) {
			QString path2=included_paths[j];
			if (!nodes.contains(path2)) {
				core->fileManager->addFile(path2);
				visible_files_node newnode;
				newnode.path=path2;
				newnode.needs_to_be_updated=true;
				nodes[path2]=newnode;
			}
			if (!nodes[path].children.contains(path2)) {
				nodes[path].children << path2;
			}
			if (nodes[path2].needs_to_be_updated)
				update_node(path2,false); //Here's the recursive part where the value of path becomes corrupted
	              }
		if (do_emit_signal) emit updated();
	}

void update_node(const QString &path_in,bool do_emit_signal) {
		QString path=path_in; //apparently this is important :(
		if (!nodes.contains(path)) return;
		if (!nodes[path].needs_to_be_updated) return;
		nodes[path].needs_to_be_updated=false;
		nodes[path].children.clear();
		QList<HaiQTag> tags=core->tagManager->byPath(path);
		QStringList search_paths=include_search_paths;
		search_paths << QFileInfo(path).path();
		QStringList included_paths;
		for (int j=0; j<tags.count(); j++) {
			if (tags[j].tagtype=="pound_include") {
				QString holdstr=get_include_file_path(tags[j].signature,search_paths);;
				if (!holdstr.isEmpty())
					included_paths << holdstr;
			}
		}
		for (int j=0; j<included_paths.count(); j++) {
			QString path2=included_paths[j];
			if (!nodes.contains(path2)) {
				core->fileManager->addFile(path2);
				visible_files_node newnode;
				newnode.path=path2;
				newnode.needs_to_be_updated=true;
				nodes[path2]=newnode;
			}
			if (!nodes[path].children.contains(path2)) {
				nodes[path].children << path2;
			}
			if (nodes[path2].needs_to_be_updated)
				update_node(path2,false); //Here's the recursive part where the value of path becomes corrupted
	              }
		if (do_emit_signal) emit updated();
	}

To copy to clipboard, switch view to plain text mode

**jacek** · 16th November 2007, 16:17

Originally Posted by magland

update_node(path2,false); //Here's the recursive part where the value of path becomes corrupted

But it's path2 not path.

What happens if you change your code to:

Qt Code:

Switch view

if (nodes[path2].needs_to_be_updated) {
   static int level = 0;
   qDebug() << "Before" << ++level << path << in_path;
   update_node(path2,false); //Here's the recursive part where the value of path becomes corrupted
   qDebug() << "After " << level-- << path << in_path;
}

if (nodes[path2].needs_to_be_updated) {
   static int level = 0;
   qDebug() << "Before" << ++level << path << in_path;
   update_node(path2,false); //Here's the recursive part where the value of path becomes corrupted
   qDebug() << "After " << level-- << path << in_path;
}

To copy to clipboard, switch view to plain text mode

?

**magland** · 16th November 2007, 18:20

Thanks, Jacek. I put in those type of debug strings as you suggested (code is at the bottom)... and now I'm really starting to be worried of a fundamental bug in the compiler.

The "path" variable changes right in the middle of the function, during a call to a completely different function. I should mention that the "update_node()" function is in a plugin library, and the implementation of the offending "addFile()" is in the main application.

Here's the log that I get out, using the two scenarios. Notice that the "path" variable changes right between debug 5 and debug 6.

Qt Code:

Switch view

LOG OUTPUT SCENARIO #1
...
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qwidget" 
1 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
0 update_node:::::10: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtcore/qdebug" 
...
 
 
LOG OUTPUT SCENARIO #2
...
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qapplication" 
1 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
0 update_node:::::10: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qwidget" 
...

LOG OUTPUT SCENARIO #1
...
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qwidget" 
1 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
0 update_node:::::10: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtcore/qdebug" 
...


LOG OUTPUT SCENARIO #2
...
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qapplication" 
1 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
0 update_node:::::10: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qwidget" 
...

To copy to clipboard, switch view to plain text mode

And here's the code that produced these two (only difference is in the top three lines):

CODE FOR SCENARIO #1

Qt Code:

Switch view

void update_node(const QString &path) {
		static int level=0;
		//QString path=path_in; //apparently this is important :(
		qDebug() << level << "update_node:::::1:" <<path;
		if (!nodes.contains(path)) return;
		if (!nodes[path].needs_to_be_updated) return;
		nodes[path].needs_to_be_updated=false;
		nodes[path].children.clear();
		QList<HaiQTag> tags=core->tagManager->byPath(path);
		emit file_tags_updated(path,tags);
		QStringList search_paths=include_search_paths;
		search_paths << QFileInfo(path).path();
		QStringList included_paths;
		qDebug() << level << "update_node:::::2:" <<path;
		for (int j=0; j<tags.count(); j++) {
			if (tags[j].tagtype=="pound_include") {
				QString holdstr=get_include_file_path(tags[j].signature,search_paths);;
				if (!holdstr.isEmpty())
					included_paths << holdstr;
			}
		}
		qDebug() << level << "update_node:::::3:" <<path;
		for (int j=0; j<included_paths.count(); j++) {
			QString path2=included_paths[j];
			qDebug() << level << "update_node:::::4:" <<path;
			if (!nodes.contains(path2)) {
				qDebug() << level << "update_node:::::5:" <<path;
				core->fileManager->addFile(path2);
				qDebug() << level << "update_node:::::6:" <<path;
				visible_files_node newnode;
				newnode.path=path2;
				newnode.needs_to_be_updated=true;
				nodes[path2]=newnode;
			}
			qDebug() << level << "update_node:::::7:" <<path;
			if ((!nodes[path].children.contains(path2))&&(path!=path2)) {
				nodes[path].children << path2;
			}
			qDebug() << level << "update_node:::::8:" <<path;
			if (nodes[path2].needs_to_be_updated) {
				qDebug() << level << "update_node:::::9:" <<path;
				level++;
				update_node(path2); 
				level--;
				qDebug() << level << "update_node:::::10:" <<path;
			}
		}
		qDebug() << level << "update_node:::::11:" << path;
	}

void update_node(const QString &path) {
		static int level=0;
		//QString path=path_in; //apparently this is important :(
		qDebug() << level << "update_node:::::1:" <<path;
		if (!nodes.contains(path)) return;
		if (!nodes[path].needs_to_be_updated) return;
		nodes[path].needs_to_be_updated=false;
		nodes[path].children.clear();
		QList<HaiQTag> tags=core->tagManager->byPath(path);
		emit file_tags_updated(path,tags);
		QStringList search_paths=include_search_paths;
		search_paths << QFileInfo(path).path();
		QStringList included_paths;
		qDebug() << level << "update_node:::::2:" <<path;
		for (int j=0; j<tags.count(); j++) {
			if (tags[j].tagtype=="pound_include") {
				QString holdstr=get_include_file_path(tags[j].signature,search_paths);;
				if (!holdstr.isEmpty())
					included_paths << holdstr;
			}
		}
		qDebug() << level << "update_node:::::3:" <<path;
		for (int j=0; j<included_paths.count(); j++) {
			QString path2=included_paths[j];
			qDebug() << level << "update_node:::::4:" <<path;
			if (!nodes.contains(path2)) {
				qDebug() << level << "update_node:::::5:" <<path;
				core->fileManager->addFile(path2);
				qDebug() << level << "update_node:::::6:" <<path;
				visible_files_node newnode;
				newnode.path=path2;
				newnode.needs_to_be_updated=true;
				nodes[path2]=newnode;
			}
			qDebug() << level << "update_node:::::7:" <<path;
			if ((!nodes[path].children.contains(path2))&&(path!=path2)) {
				nodes[path].children << path2;
			}
			qDebug() << level << "update_node:::::8:" <<path;
			if (nodes[path2].needs_to_be_updated) {
				qDebug() << level << "update_node:::::9:" <<path;
				level++;
				update_node(path2); 
				level--;
				qDebug() << level << "update_node:::::10:" <<path;
			}
		}
		qDebug() << level << "update_node:::::11:" << path;
	}

To copy to clipboard, switch view to plain text mode

CODE FOR SCENARIO #2

Qt Code:

Switch view

void update_node(const QString &path_in) {
		static int level=0;
		QString path=path_in; //apparently this is important :(
		qDebug() << level << "update_node:::::1:" <<path;
		if (!nodes.contains(path)) return;
		if (!nodes[path].needs_to_be_updated) return;
		nodes[path].needs_to_be_updated=false;
		nodes[path].children.clear();
		QList<HaiQTag> tags=core->tagManager->byPath(path);
		emit file_tags_updated(path,tags);
		QStringList search_paths=include_search_paths;
		search_paths << QFileInfo(path).path();
		QStringList included_paths;
		qDebug() << level << "update_node:::::2:" <<path;
		for (int j=0; j<tags.count(); j++) {
			if (tags[j].tagtype=="pound_include") {
				QString holdstr=get_include_file_path(tags[j].signature,search_paths);;
				if (!holdstr.isEmpty())
					included_paths << holdstr;
			}
		}
		qDebug() << level << "update_node:::::3:" <<path;
		for (int j=0; j<included_paths.count(); j++) {
			QString path2=included_paths[j];
			qDebug() << level << "update_node:::::4:" <<path;
			if (!nodes.contains(path2)) {
				qDebug() << level << "update_node:::::5:" <<path;
				core->fileManager->addFile(path2);
				qDebug() << level << "update_node:::::6:" <<path;
				visible_files_node newnode;
				newnode.path=path2;
				newnode.needs_to_be_updated=true;
				nodes[path2]=newnode;
			}
			qDebug() << level << "update_node:::::7:" <<path;
			if ((!nodes[path].children.contains(path2))&&(path!=path2)) {
				nodes[path].children << path2;
			}
			qDebug() << level << "update_node:::::8:" <<path;
			if (nodes[path2].needs_to_be_updated) {
				qDebug() << level << "update_node:::::9:" <<path;
				level++;
				update_node(path2); 
				level--;
				qDebug() << level << "update_node:::::10:" <<path;
			}
		}
		qDebug() << level << "update_node:::::11:" << path;
	}

void update_node(const QString &path_in) {
		static int level=0;
		QString path=path_in; //apparently this is important :(
		qDebug() << level << "update_node:::::1:" <<path;
		if (!nodes.contains(path)) return;
		if (!nodes[path].needs_to_be_updated) return;
		nodes[path].needs_to_be_updated=false;
		nodes[path].children.clear();
		QList<HaiQTag> tags=core->tagManager->byPath(path);
		emit file_tags_updated(path,tags);
		QStringList search_paths=include_search_paths;
		search_paths << QFileInfo(path).path();
		QStringList included_paths;
		qDebug() << level << "update_node:::::2:" <<path;
		for (int j=0; j<tags.count(); j++) {
			if (tags[j].tagtype=="pound_include") {
				QString holdstr=get_include_file_path(tags[j].signature,search_paths);;
				if (!holdstr.isEmpty())
					included_paths << holdstr;
			}
		}
		qDebug() << level << "update_node:::::3:" <<path;
		for (int j=0; j<included_paths.count(); j++) {
			QString path2=included_paths[j];
			qDebug() << level << "update_node:::::4:" <<path;
			if (!nodes.contains(path2)) {
				qDebug() << level << "update_node:::::5:" <<path;
				core->fileManager->addFile(path2);
				qDebug() << level << "update_node:::::6:" <<path;
				visible_files_node newnode;
				newnode.path=path2;
				newnode.needs_to_be_updated=true;
				nodes[path2]=newnode;
			}
			qDebug() << level << "update_node:::::7:" <<path;
			if ((!nodes[path].children.contains(path2))&&(path!=path2)) {
				nodes[path].children << path2;
			}
			qDebug() << level << "update_node:::::8:" <<path;
			if (nodes[path2].needs_to_be_updated) {
				qDebug() << level << "update_node:::::9:" <<path;
				level++;
				update_node(path2); 
				level--;
				qDebug() << level << "update_node:::::10:" <<path;
			}
		}
		qDebug() << level << "update_node:::::11:" << path;
	}

To copy to clipboard, switch view to plain text mode

and here is the declaration of addFile...

Qt Code:

Switch view

void addFile(const QString &path);

void addFile(const QString &path);

To copy to clipboard, switch view to plain text mode

**jacek** · 16th November 2007, 22:47

Originally Posted by magland

The "path" variable changes right in the middle of the function, during a call to a completely different function.

This means that we have to take a look at this completely different function. It isn't time for worrying yet.

Probably your code does something similar to this:

Qt Code:

Switch view

#include <iostream>
 
int z = 0;
 
void bar( int y )
{
   z = y;
}
 
void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}
 
int main()
{
   foo( z );
   return 0;
}

#include <iostream>

int z = 0;

void bar( int y )
{
   z = y;
}

void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}

int main()
{
   foo( z );
   return 0;
}

To copy to clipboard, switch view to plain text mode

**magland** · 17th November 2007, 11:09

Ok, I see what you are saying. Now I see the danger of passing things by reference (even if declared const). The advantage is that the parameter does not need to be copied, but the tradeoff is that you need to worry about whether the variable is changed part way through the execution of the function - not within the function but elsewhere.

This is probably what was happening to me. But I will never know, because I've restructured my program to avoid the recursion (I'm now using a queuing system to do the same task).

Based on a recent recommendation on this forum I changed my coding habits and use "const QString &" everywhere instead of "QString".... but now I see that I need to be more careful. Do you have a recommendation of what precautions I need to take with this?

Thanks!

Originally Posted by jacek

This means that we have to take a look at this completely different function. It isn't time for worrying yet.

Probably your code does something similar to this:

Qt Code:

Switch view

#include <iostream>
 
int z = 0;
 
void bar( int y )
{
   z = y;
}
 
void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}
 
int main()
{
   foo( z );
   return 0;
}

#include <iostream>

int z = 0;

void bar( int y )
{
   z = y;
}

void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}

int main()
{
   foo( z );
   return 0;
}

To copy to clipboard, switch view to plain text mode

**Gopala Krishna** · 20th November 2007, 12:55

Originally Posted by jacek

This means that we have to take a look at this completely different function. It isn't time for worrying yet.

Probably your code does something similar to this:

Qt Code:

Switch view

#include <iostream>
 
int z = 0;
 
void bar( int y )
{
   z = y;
}
 
void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}
 
int main()
{
   foo( z );
   return 0;
}

#include <iostream>

int z = 0;

void bar( int y )
{
   z = y;
}

void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}

int main()
{
   foo( z );
   return 0;
}

To copy to clipboard, switch view to plain text mode

Very cool observation jacek :-)

So basically in the code posted by magland, it seems that the parameter passed is a class member variable(as there is an emit statement, the func should be a class member) which is modified somewhere may be in a slot.
The actual answer to whether the reference form should be used or not is a dependent question. It depends on whether you modify the parameter sent outside the function when the function with const reference is not yet returned.

Anyway, many thanks Jacek as well as magland for exposing such a kind of potential bug.
A lesson learnt well in the right time

**jacek** · 28th November 2007, 00:26

Originally Posted by magland

Based on a recent recommendation on this forum I changed my coding habits and use "const QString &" everywhere instead of "QString".... but now I see that I need to be more careful.

It's recommended to pass objects by reference to avoid copying, but on the other hand in Qt it's cheap to copy implicitly shared classes.

Originally Posted by magland

Do you have a recommendation of what precautions I need to take with this?

I was thinking about this for few days and I couldn't come up with any rules. You situation is a bit similar to:

Qt Code:

Switch view

QVector v;
...
v.append( v[0] );

QVector v;
...
v.append( v[0] );

To copy to clipboard, switch view to plain text mode

The argument is passed as reference, but it must be copied when QVector needs to reallocate data. You can easily identify such situation in a single data structure, but the problem arises when the data is passed between many classes. But I would simply classify that as one of those Bad Things(tm) that might happen when you use global variables (or you have cycles in call graph).

**magland** · 28th November 2007, 15:34

Thanks Jacek. I agree with you.

After finding a number of subtle problems caused by passing by reference (in addition to the one I reported) I have concluded that it is important to NEVER pass arguments by reference UNLESS you can guarantee that the function does not access global variables, or "have cycles in the call graph" that may end up modifying the input variable. It is just not worth the danger! There's nothing worse than problems that occur mysteriously once in every 1000 calls.

Of course, passing by reference is good, if you can ensure such things. I don't use global variables, but I do use a model/view framework which inevitably will have some cycles or recursive calls. Therefore, I need to be careful about it.

Thread: Disturbing: const QString &

Thread Tools

Search Thread

Display

Disturbing: const QString &

Re: Disturbing: const QString &

Re: Disturbing: const QString &

Re: Disturbing: const QString &

Re: Disturbing: const QString &

Re: Disturbing: const QString &

The following 2 users say thank you to jacek for this useful post:

Re: Disturbing: const QString &

The following user says thank you to magland for this useful post:

Re: Disturbing: const QString &

Re: Disturbing: const QString &

Re: Disturbing: const QString &

Similar Threads

qt 4.2.2 install in tru64 cxx

Convert from iso-8859-1 to... Something else :-)

QTableView paints too much

Delegates and Table

[4.1.1] Custom Widget visible in Edit but not in Preview

Bookmarks

Bookmarks

Posting Permissions