QFileDialog::getOpenFileNames crashes [solved]

Message

DaggyStyle · Post by **DaggyStyle** » Tue Oct 26, 2010 9:08 pm

hello, here is another problem that I've encountered I have this line: http://gitorious.org/open-source-soccer ... pp#line101 if that line stays there or goes to any function within TrainingActivityManager.cpp (which calls that function) or to each private slot function within MainScreen.cpp (which instantiates that TrainingActivityManager dialog), the program crashes, if I put it in any other functions, it works ok.
why?

avendesora · Post by **avendesora** » Wed Oct 27, 2010 6:29 pm

Hi,

Can't even begin to compile your code. Using #warnings for todo items is a Real Bad Idea (tm) IMO.
You're pretty much drowning your logs with stuff and overlooking potentially killer items like:

Code: Select all

ossm/src/Threads/TubeWrapper.cpp:10: warning: no return statement in function returning non-void

You're in undefined behavior territory if at any point something somewhere uses that (non-existent) return value.
(And that's not the only one.)

Try and make you code build cleanly with -Wall -Werror, then make it compile at -Wall -Wextra -Werror, and slap in -pedantic while you're at it.
If you're worried about data type sizes in some places, slap in -Wconversion (that one's hard though).

For your TODO's, notes to self and such, just use fixed format comments '//TODO: whatever', '//WARNING: grue' and use git grep to find them all when you need to.

(BTW, stable GCC-4.4 is 4.4.4, not 4.4.5.)

DaggyStyle · Post by **DaggyStyle** » Wed Oct 27, 2010 8:27 pm

avendesora wrote:Hi,

Can't even begin to compile your code. Using #warnings for todo items is a Real Bad Idea (tm) IMO.
You're pretty much drowning your logs with stuff and overlooking potentially killer items like:
Code: Select all
ossm/src/Threads/TubeWrapper.cpp:10: warning: no return statement in function returning non-void
You're in undefined behavior territory if at any point something somewhere uses that (non-existent) return value.
(And that's not the only one.)

forgot about that, thanks.

avendesora wrote: Try and make you code build cleanly with -Wall -Werror, then make it compile at -Wall -Wextra -Werror, and slap in -pedantic while you're at it.
If you're worried about data type sizes in some places, slap in -Wconversion (that one's hard though).

done, no visible effect.

avendesora wrote: For your TODO's, notes to self and such, just use fixed format comments '//TODO: whatever', '//WARNING: grue' and use git grep to find them all when you need to.

done

avendesora wrote:(BTW, stable GCC-4.4 is 4.4.4, not 4.4.5.)

I have no problems with 4.4.5, also it isn't gentoo oriented.

still beside that, any ideas how to solve my problem?

DaggyStyle · Post by **DaggyStyle** » Wed Oct 27, 2010 8:33 pm

-werror is no good, it's complains about boost errors. I've thrown it out.

avendesora · Post by **avendesora** » Wed Oct 27, 2010 9:59 pm

Well, maybe push those changes to your repo so people can try them out.
Right now ignoring the compile errors, the only things I get when starting "ossm" is a hard crash when clicking any of the active buttons (bzip2_error exception raised).

Post by Hu » Thu Oct 28, 2010 2:33 am

DaggyStyle wrote:-werror is no good, it's complains about boost errors. I've thrown it out.

What boost errors? Have you reported them? Major libraries ought to be warning-free, and the Boost project has been pretty good about correcting warnings that are reported to them.

DaggyStyle · Post by **DaggyStyle** » Thu Oct 28, 2010 5:15 pm

avendesora wrote:Well, maybe push those changes to your repo so people can try them out.
Right now ignoring the compile errors, the only things I get when starting "ossm" is a hard crash when clicking any of the active buttons (bzip2_error exception raised).

I did, do the following:

pull head.
compile
under build, create a link called data that points to data folder.
copy Config.xml to build
run again.

Hu wrote:
DaggyStyle wrote:-werror is no good, it's complains about boost errors. I've thrown it out.
What boost errors? Have you reported them? Major libraries ought to be warning-free, and the Boost project has been pretty good about correcting warnings that are reported to them.

here is the output:

Code: Select all

/home/dagg/workspace/OSSM/build> make -j3
/usr/bin/cmake -H/home/dagg/workspace/OSSM -B/home/dagg/workspace/OSSM/build --check-build-system CMakeFiles/Makefile.cmake 0
--   Found c++0x support
-- Test flag is enabled, caching testunits files...
--   adding test unit for testUnitsUtils
--   adding test unit for testUnitsStreamHandlers
-- Configuring done
-- Generating done
-- Build files have been written to: /home/dagg/workspace/OSSM/build
/usr/bin/cmake -E cmake_progress_start /home/dagg/workspace/OSSM/build/CMakeFiles /home/dagg/workspace/OSSM/build/CMakeFiles/progress.marks
/usr/bin/make -f CMakeFiles/Makefile2 all
make[1]: Entering directory `/home/dagg/workspace/OSSM/build'
/usr/bin/make -f src/DataObjects/CMakeFiles/DataObjects.dir/build.make src/DataObjects/CMakeFiles/DataObjects.dir/depend
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
cd /home/dagg/workspace/OSSM/build && /usr/bin/cmake -E cmake_depends "Unix Makefiles" /home/dagg/workspace/OSSM /home/dagg/workspace/OSSM/src/DataObjects /home/dagg/workspace/OSSM/build /home/dagg/workspace/OSSM/build/src/DataObjects /home/dagg/workspace/OSSM/build/src/DataObjects/CMakeFiles/DataObjects.dir/DependInfo.cmake --color=
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/make -f src/DataObjects/CMakeFiles/DataObjects.dir/build.make src/DataObjects/CMakeFiles/DataObjects.dir/build
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 1
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 2
[  2%] [  4%] Building CXX object src/DataObjects/CMakeFiles/DataObjects.dir/City.cpp.o
linking CMakeFiles/DataObjects.dir/City.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 3
Building CXX object src/DataObjects/CMakeFiles/DataObjects.dir/Competition.cpp.o
linking CMakeFiles/DataObjects.dir/Competition.cpp.o (g++)
[  6%] Building CXX object src/DataObjects/CMakeFiles/DataObjects.dir/Continent.cpp.o
linking CMakeFiles/DataObjects.dir/Continent.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 4
[  8%] Building CXX object src/DataObjects/CMakeFiles/DataObjects.dir/Language.cpp.o
linking CMakeFiles/DataObjects.dir/Language.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 5
[ 10%] Building CXX object src/DataObjects/CMakeFiles/DataObjects.dir/Nation.cpp.o
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 6
linking CMakeFiles/DataObjects.dir/Nation.cpp.o (g++)
[ 12%] Building CXX object src/DataObjects/CMakeFiles/DataObjects.dir/Stadium.cpp.o
linking CMakeFiles/DataObjects.dir/Stadium.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 7
[ 14%] Building CXX object src/DataObjects/CMakeFiles/DataObjects.dir/Staff.cpp.o
linking CMakeFiles/DataObjects.dir/Staff.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 8
[ 16%] Building CXX object src/DataObjects/CMakeFiles/DataObjects.dir/Team.cpp.o
linking CMakeFiles/DataObjects.dir/Team.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 9
[ 18%] Building CXX object src/DataObjects/CMakeFiles/DataObjects.dir/TreeNode.cpp.o
linking CMakeFiles/DataObjects.dir/TreeNode.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 10
[ 20%] Building CXX object src/DataObjects/CMakeFiles/DataObjects.dir/TubeSystemDB.cpp.o
linking CMakeFiles/DataObjects.dir/TubeSystemDB.cpp.o (g++)
Linking CXX shared library ../../libs/libDataObjects.so
cd /home/dagg/workspace/OSSM/build/src/DataObjects && /usr/bin/cmake -E cmake_link_script CMakeFiles/DataObjects.dir/link.txt --verbose=1
linking ../../libs/libDataObjects.so (g++)
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles  1 2 3 4 5 6 7 8 9 10
[ 20%] Built target DataObjects
/usr/bin/make -f src/Utilities/CMakeFiles/Utilities.dir/build.make src/Utilities/CMakeFiles/Utilities.dir/depend
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
cd /home/dagg/workspace/OSSM/build && /usr/bin/cmake -E cmake_depends "Unix Makefiles" /home/dagg/workspace/OSSM /home/dagg/workspace/OSSM/src/Utilities /home/dagg/workspace/OSSM/build /home/dagg/workspace/OSSM/build/src/Utilities /home/dagg/workspace/OSSM/build/src/Utilities/CMakeFiles/Utilities.dir/DependInfo.cmake --color=
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/make -f src/Utilities/CMakeFiles/Utilities.dir/build.make src/Utilities/CMakeFiles/Utilities.dir/build
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 38
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 39
[ 22%] [ 24%] /usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 40
Building CXX object src/Utilities/CMakeFiles/Utilities.dir/Base64Encoder.cpp.o
linking CMakeFiles/Utilities.dir/Base64Encoder.cpp.o (g++)
[ 26%] Building CXX object src/Utilities/CMakeFiles/Utilities.dir/Base64Decoder.cpp.o
Building CXX object src/Utilities/CMakeFiles/Utilities.dir/ConfInstance.cpp.o
linking CMakeFiles/Utilities.dir/Base64Decoder.cpp.o (g++)
linking CMakeFiles/Utilities.dir/ConfInstance.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 41
[ 28%] Building CXX object src/Utilities/CMakeFiles/Utilities.dir/DataFactory.cpp.o
linking CMakeFiles/Utilities.dir/DataFactory.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 42
[ 30%] Building CXX object src/Utilities/CMakeFiles/Utilities.dir/Logger.cpp.o
linking CMakeFiles/Utilities.dir/Logger.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 43
[ 32%] Building CXX object src/Utilities/CMakeFiles/Utilities.dir/TubesPool.cpp.o
linking CMakeFiles/Utilities.dir/TubesPool.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 44
[ 34%] Building CXX object src/Utilities/CMakeFiles/Utilities.dir/Utils-DataManipulation.cpp.o
linking CMakeFiles/Utilities.dir/Utils-DataManipulation.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 45
[ 36%] Building CXX object src/Utilities/CMakeFiles/Utilities.dir/Utils-General.cpp.o
linking CMakeFiles/Utilities.dir/Utils-General.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 46
[ 38%] Building CXX object src/Utilities/CMakeFiles/Utilities.dir/Utils-MemoryCell.cpp.o
linking CMakeFiles/Utilities.dir/Utils-MemoryCell.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 47
[ 40%] Building CXX object src/Utilities/CMakeFiles/Utilities.dir/Utils-StaffDlg.cpp.o
linking CMakeFiles/Utilities.dir/Utils-StaffDlg.cpp.o (g++)
Linking CXX shared library ../../libs/libUtilities.so
cd /home/dagg/workspace/OSSM/build/src/Utilities && /usr/bin/cmake -E cmake_link_script CMakeFiles/Utilities.dir/link.txt --verbose=1
linking ../../libs/libUtilities.so (g++)
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles  38 39 40 41 42 43 44 45 46 47
[ 40%] Built target Utilities
/usr/bin/make -f src/Threads/CMakeFiles/Threads.dir/build.make src/Threads/CMakeFiles/Threads.dir/depend
/usr/bin/make -f src/GUI/CMakeFiles/GUI.dir/build.make src/GUI/CMakeFiles/GUI.dir/depend
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
cd /home/dagg/workspace/OSSM/build && /usr/bin/cmake -E cmake_depends "Unix Makefiles" /home/dagg/workspace/OSSM /home/dagg/workspace/OSSM/src/Threads /home/dagg/workspace/OSSM/build /home/dagg/workspace/OSSM/build/src/Threads /home/dagg/workspace/OSSM/build/src/Threads/CMakeFiles/Threads.dir/DependInfo.cmake --color=
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/make -f src/Threads/CMakeFiles/Threads.dir/build.make src/Threads/CMakeFiles/Threads.dir/build
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 34
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 35
[ 42%] [ 44%] Building CXX object src/Threads/CMakeFiles/Threads.dir/AgingThread.cpp.o
linking CMakeFiles/Threads.dir/AgingThread.cpp.o (g++)
Building CXX object src/Threads/CMakeFiles/Threads.dir/GameThread.cpp.o
linking CMakeFiles/Threads.dir/GameThread.cpp.o (g++)
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 33
[ 46%] Generating ui_TrainingActivityManager.h
generating /home/dagg/workspace/OSSM/build/src/GUI/ui_TrainingActivityManager.h (uic)
Warning: name layoutWidget is already used
Warning: name layoutWidget is already used
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 24
[ 48%] Generating moc_MainScreen.cxx
generating /home/dagg/workspace/OSSM/build/src/GUI/moc_MainScreen.cxx (moc)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 36
[ 50%] Building CXX object src/Threads/CMakeFiles/Threads.dir/RecordReplaceingDaemon.cpp.o
linking CMakeFiles/Threads.dir/RecordReplaceingDaemon.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 25
[ 52%] Generating moc_ProgressBar.cxx
generating /home/dagg/workspace/OSSM/build/src/GUI/moc_ProgressBar.cxx (moc)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 26
[ 54%] Generating moc_QStringSpinBox.cxx
generating /home/dagg/workspace/OSSM/build/src/GUI/moc_QStringSpinBox.cxx (moc)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 27
[ 56%] Generating moc_StaffDlg.cxx
generating /home/dagg/workspace/OSSM/build/src/GUI/moc_StaffDlg.cxx (moc)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 28
[ 58%] Generating moc_TableComboBoxItem.cxx
generating /home/dagg/workspace/OSSM/build/src/GUI/moc_TableComboBoxItem.cxx (moc)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 29
[ 60%] Generating moc_TrainingActivityManager.cxx
generating /home/dagg/workspace/OSSM/build/src/GUI/moc_TrainingActivityManager.cxx (moc)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 30
[ 62%] Generating ui_MainScreen.h
generating /home/dagg/workspace/OSSM/build/src/GUI/ui_MainScreen.h (uic)
Warning: name layoutWidget is already used
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 31
[ 64%] Generating ui_ProgressBar.h
generating /home/dagg/workspace/OSSM/build/src/GUI/ui_ProgressBar.h (uic)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 32
[ 66%] Generating ui_StaffDlg.h
generating /home/dagg/workspace/OSSM/build/src/GUI/ui_StaffDlg.h (uic)
cd /home/dagg/workspace/OSSM/build && /usr/bin/cmake -E cmake_depends "Unix Makefiles" /home/dagg/workspace/OSSM /home/dagg/workspace/OSSM/src/GUI /home/dagg/workspace/OSSM/build /home/dagg/workspace/OSSM/build/src/GUI /home/dagg/workspace/OSSM/build/src/GUI/CMakeFiles/GUI.dir/DependInfo.cmake --color=
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/make -f src/GUI/CMakeFiles/GUI.dir/build.make src/GUI/CMakeFiles/GUI.dir/build
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 11
[ 68%] Building CXX object src/GUI/CMakeFiles/GUI.dir/MainScreen.cpp.o
linking CMakeFiles/GUI.dir/MainScreen.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 12
[ 70%] Building CXX object src/GUI/CMakeFiles/GUI.dir/ProgressBar.cpp.o
linking CMakeFiles/GUI.dir/ProgressBar.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 37
[ 72%] Building CXX object src/Threads/CMakeFiles/Threads.dir/TubeWrapper.cpp.o
linking CMakeFiles/Threads.dir/TubeWrapper.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 13
[ 74%] Building CXX object src/GUI/CMakeFiles/GUI.dir/QStringSpinBox.cpp.o
linking CMakeFiles/GUI.dir/QStringSpinBox.cpp.o (g++)
Linking CXX shared library ../../libs/libThreads.so
cd /home/dagg/workspace/OSSM/build/src/Threads && /usr/bin/cmake -E cmake_link_script CMakeFiles/Threads.dir/link.txt --verbose=1
linking ../../libs/libThreads.so (g++)
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles  34 35 36 37
[ 74%] Built target Threads
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 14
[ 76%] Building CXX object src/GUI/CMakeFiles/GUI.dir/StaffDlg.cpp.o
linking CMakeFiles/GUI.dir/StaffDlg.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 15
[ 78%] Building CXX object src/GUI/CMakeFiles/GUI.dir/TableComboBoxItem.cpp.o
linking CMakeFiles/GUI.dir/TableComboBoxItem.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 16
[ 80%] Building CXX object src/GUI/CMakeFiles/GUI.dir/TrainingActivityManager.cpp.o
linking CMakeFiles/GUI.dir/TrainingActivityManager.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 17
[ 82%] Building CXX object src/GUI/CMakeFiles/GUI.dir/Util-Gui.cpp.o
linking CMakeFiles/GUI.dir/Util-Gui.cpp.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 18
[ 84%] Building CXX object src/GUI/CMakeFiles/GUI.dir/moc_MainScreen.cxx.o
linking CMakeFiles/GUI.dir/moc_MainScreen.cxx.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 19
[ 86%] Building CXX object src/GUI/CMakeFiles/GUI.dir/moc_ProgressBar.cxx.o
linking CMakeFiles/GUI.dir/moc_ProgressBar.cxx.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 20
[ 88%] Building CXX object src/GUI/CMakeFiles/GUI.dir/moc_QStringSpinBox.cxx.o
linking CMakeFiles/GUI.dir/moc_QStringSpinBox.cxx.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 21
[ 90%] Building CXX object src/GUI/CMakeFiles/GUI.dir/moc_StaffDlg.cxx.o
linking CMakeFiles/GUI.dir/moc_StaffDlg.cxx.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 22
[ 92%] Building CXX object src/GUI/CMakeFiles/GUI.dir/moc_TableComboBoxItem.cxx.o
linking CMakeFiles/GUI.dir/moc_TableComboBoxItem.cxx.o (g++)
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 23
[ 94%] Building CXX object src/GUI/CMakeFiles/GUI.dir/moc_TrainingActivityManager.cxx.o
linking CMakeFiles/GUI.dir/moc_TrainingActivityManager.cxx.o (g++)
Linking CXX shared library ../../libs/libGUI.so
cd /home/dagg/workspace/OSSM/build/src/GUI && /usr/bin/cmake -E cmake_link_script CMakeFiles/GUI.dir/link.txt --verbose=1
linking ../../libs/libGUI.so (g++)
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles  11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33
[ 94%] Built target GUI
/usr/bin/make -f src/CMakeFiles/ossm.dir/build.make src/CMakeFiles/ossm.dir/depend
/usr/bin/make -f src/testUnits/CMakeFiles/testUnitsStreamHandlers.dir/build.make src/testUnits/CMakeFiles/testUnitsStreamHandlers.dir/depend
/usr/bin/make -f src/testUnits/CMakeFiles/testUnitsUtils.dir/build.make src/testUnits/CMakeFiles/testUnitsUtils.dir/depend
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
cd /home/dagg/workspace/OSSM/build && /usr/bin/cmake -E cmake_depends "Unix Makefiles" /home/dagg/workspace/OSSM /home/dagg/workspace/OSSM/src /home/dagg/workspace/OSSM/build /home/dagg/workspace/OSSM/build/src /home/dagg/workspace/OSSM/build/src/CMakeFiles/ossm.dir/DependInfo.cmake --color=
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
cd /home/dagg/workspace/OSSM/build && /usr/bin/cmake -E cmake_depends "Unix Makefiles" /home/dagg/workspace/OSSM /home/dagg/workspace/OSSM/src/testUnits /home/dagg/workspace/OSSM/build /home/dagg/workspace/OSSM/build/src/testUnits /home/dagg/workspace/OSSM/build/src/testUnits/CMakeFiles/testUnitsStreamHandlers.dir/DependInfo.cmake --color=
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
cd /home/dagg/workspace/OSSM/build && /usr/bin/cmake -E cmake_depends "Unix Makefiles" /home/dagg/workspace/OSSM /home/dagg/workspace/OSSM/src/testUnits /home/dagg/workspace/OSSM/build /home/dagg/workspace/OSSM/build/src/testUnits /home/dagg/workspace/OSSM/build/src/testUnits/CMakeFiles/testUnitsUtils.dir/DependInfo.cmake --color=
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/make -f src/CMakeFiles/ossm.dir/build.make src/CMakeFiles/ossm.dir/build
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/make -f src/testUnits/CMakeFiles/testUnitsStreamHandlers.dir/build.make src/testUnits/CMakeFiles/testUnitsStreamHandlers.dir/build
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/make -f src/testUnits/CMakeFiles/testUnitsUtils.dir/build.make src/testUnits/CMakeFiles/testUnitsUtils.dir/build
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 48
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 49
[ 96%] [ 98%] Building CXX object src/CMakeFiles/ossm.dir/Ossm.cpp.o
linking CMakeFiles/ossm.dir/Ossm.cpp.o (g++)
Building CXX object src/testUnits/CMakeFiles/testUnitsStreamHandlers.dir/testUnitsStreamHandlers.cpp.o
linking CMakeFiles/testUnitsStreamHandlers.dir/testUnitsStreamHandlers.cpp.o (g++)
make[2]: Entering directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles 50
[100%] Building CXX object src/testUnits/CMakeFiles/testUnitsUtils.dir/testUnitsUtils.cpp.o
linking CMakeFiles/testUnitsUtils.dir/testUnitsUtils.cpp.o (g++)
cc1plus: warnings being treated as errors
In file included from /usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:17,
                 from /usr/include/boost-1_42/boost/thread/xtime.hpp:15,
                 from /usr/include/boost-1_42/boost/thread/pthread/mutex.hpp:13,
                 from /usr/include/boost-1_42/boost/thread/mutex.hpp:16,
                 from /usr/include/boost-1_42/boost/thread/pthread/shared_mutex.hpp:12,
                 from /usr/include/boost-1_42/boost/thread/shared_mutex.hpp:16,
                 from /home/dagg/workspace/OSSM/src/Utilities/BaseDefinitions.hpp:29,
                 from /home/dagg/workspace/OSSM/src/DataObjects/TubeSystemDB.hpp:29,
                 from /home/dagg/workspace/OSSM/src/Threads/TubeWrapper.hpp:29,
                 from /home/dagg/workspace/OSSM/src/Utilities/TubesPool.hpp:29,
                 from /home/dagg/workspace/OSSM/src/Ossm.hpp:29,
                 from /home/dagg/workspace/OSSM/src/Ossm.cpp:1:
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp: In function ‘tm boost::gregorian::to_tm(const boost::gregorian::date&)’:
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp:44: error: missing initializer for member ‘tm::tm_sec’
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp:44: error: missing initializer for member ‘tm::tm_min’
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp:44: error: missing initializer for member ‘tm::tm_hour’
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp:44: error: missing initializer for member ‘tm::tm_mday’
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp:44: error: missing initializer for member ‘tm::tm_mon’
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp:44: error: missing initializer for member ‘tm::tm_year’
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp:44: error: missing initializer for member ‘tm::tm_wday’
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp:44: error: missing initializer for member ‘tm::tm_yday’
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp:44: error: missing initializer for member ‘tm::tm_isdst’
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp:44: error: missing initializer for member ‘tm::tm_gmtoff’
/usr/include/boost-1_42/boost/date_time/gregorian/conversion.hpp:44: error: missing initializer for member ‘tm::tm_zone’
In file included from /usr/include/boost-1_42/boost/thread/xtime.hpp:15,
                 from /usr/include/boost-1_42/boost/thread/pthread/mutex.hpp:13,
                 from /usr/include/boost-1_42/boost/thread/mutex.hpp:16,
                 from /usr/include/boost-1_42/boost/thread/pthread/shared_mutex.hpp:12,
                 from /usr/include/boost-1_42/boost/thread/shared_mutex.hpp:16,
                 from /home/dagg/workspace/OSSM/src/Utilities/BaseDefinitions.hpp:29,
                 from /home/dagg/workspace/OSSM/src/DataObjects/TubeSystemDB.hpp:29,
                 from /home/dagg/workspace/OSSM/src/Threads/TubeWrapper.hpp:29,
                 from /home/dagg/workspace/OSSM/src/Utilities/TubesPool.hpp:29,
                 from /home/dagg/workspace/OSSM/src/Ossm.hpp:29,
                 from /home/dagg/workspace/OSSM/src/Ossm.cpp:1:
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp: In function ‘tm boost::posix_time::to_tm(const boost::posix_time::time_duration&)’:
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:46: error: missing initializer for member ‘tm::tm_sec’
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:46: error: missing initializer for member ‘tm::tm_min’
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:46: error: missing initializer for member ‘tm::tm_hour’
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:46: error: missing initializer for member ‘tm::tm_mday’
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:46: error: missing initializer for member ‘tm::tm_mon’
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:46: error: missing initializer for member ‘tm::tm_year’
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:46: error: missing initializer for member ‘tm::tm_wday’
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:46: error: missing initializer for member ‘tm::tm_yday’
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:46: error: missing initializer for member ‘tm::tm_isdst’
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:46: error: missing initializer for member ‘tm::tm_gmtoff’
/usr/include/boost-1_42/boost/date_time/posix_time/conversion.hpp:46: error: missing initializer for member ‘tm::tm_zone’
In file included from /home/dagg/workspace/OSSM/src/DataObjects/Team.hpp:30,
                 from /home/dagg/workspace/OSSM/src/Utilities/DataFactory.hpp:33,
                 from /home/dagg/workspace/OSSM/src/Ossm.cpp:4:
/home/dagg/workspace/OSSM/src/DataObjects/Contract.hpp: At global scope:
/home/dagg/workspace/OSSM/src/DataObjects/Contract.hpp:51: error: unused parameter ‘other’
make[2]: *** [src/CMakeFiles/ossm.dir/Ossm.cpp.o] Error 1
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
make[1]: *** [src/CMakeFiles/ossm.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
Linking CXX executable ../../testUnitsUtils
cd /home/dagg/workspace/OSSM/build/src/testUnits && /usr/bin/cmake -E cmake_link_script CMakeFiles/testUnitsUtils.dir/link.txt --verbose=1
linking ../../testUnitsUtils (g++)
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles  50
[100%] Built target testUnitsUtils
Linking CXX executable ../../testUnitsStreamHandlers
cd /home/dagg/workspace/OSSM/build/src/testUnits && /usr/bin/cmake -E cmake_link_script CMakeFiles/testUnitsStreamHandlers.dir/link.txt --verbose=1
linking ../../testUnitsStreamHandlers (g++)
make[2]: Leaving directory `/home/dagg/workspace/OSSM/build'
/usr/bin/cmake -E cmake_progress_report /home/dagg/workspace/OSSM/build/CMakeFiles  49
[100%] Built target testUnitsStreamHandlers
make[1]: Leaving directory `/home/dagg/workspace/OSSM/build'
make: *** [all] Error 2
*** Failed ***

rtomek · Post by **rtomek** » Thu Oct 28, 2010 7:08 pm

those warnings were fixed with boost 1.43 (which isn't in portage)

DaggyStyle · Post by **DaggyStyle** » Thu Oct 28, 2010 7:45 pm

rtomek wrote:those warnings were fixed with boost 1.43 (which isn't in portage)

that is the main problem, boost has couple of problems that are fixed in newer version but the portage version is lagging quite a bit.

avendesora · Post by **avendesora** » Sat Oct 30, 2010 11:06 am

Code: Select all

const bool operator==(const Continent& other);

This is not the correct signature. The 'const' goes at the end. (Same thing for a lot of classes.)

You're declaring and defining a bunch of operator= methods that don't do any assignment.
If you don't want the objects to be copied/assigned, don't define the methods so that you get a link time error rather than code that silently does not do what you expect it to at all. (Same thing for the dummy copy constructors.)

In the MemoryCell thingy, you have this type of construct:

Code: Select all

return (1==(vector[index]->bits | 4));

I'm pretty sure this will always be false. (whatever|4 >= 4.)
To test a bit: (whatever & (1<<bitnumber)) != 0.

Code: Select all

const boost::uint8_t& pSkillFitness(const staffPtr obj);

and all the others: C-style pointer casts == bad. That whole file is a mess. Why don't you do something like an enum for the various skills (one for each 'type' of skill), and a single accessor? You should not need those casts at all.

Code: Select all

const std::string Utils::General::getRep(const std::map< std::pair< boost::uint8_t, boost::uint8_t >, std::string >& strDataMap, const boost::uint8_t& num) {
  std::string ans("Null");
  if (num<252) {
    bool stop=false;
    for (std::map< std::pair< boost::uint8_t, boost::uint8_t >, std::string >::const_iterator it=strDataMap.begin();strDataMap.end()!=it and !stop;it++) {
      std::pair< const std::pair< boost::uint8_t, boost::uint8_t >, std::string > item=*it;
      if ((item.first.first<=num) and (item.first.second>=num)) {
    ans=item.second;
    stop=true;
      }
    }
  }
  return ans;
}

Ahh my head hurts

typedef that bloody map type and use the auto keyword.

Code: Select all

const std::string getRep(const supermap& dataMap, const uint8_t& num) {
 if (num < 252) {
  for (auto it = dataMap.begin(); it != dataMap.end(); ++it)
   if (((*it).first.first <= num) &&  ((*it).first.second >= num)))
    return (*it).second;
 }
 return <default>
}

BTW, why take a const uint8_t& as an argument? Why return a const string? (That method could be const though.)

You still have non-void code that doesn't return anything.

Still getting bzip2_error exception when clicking on new game.

Post by Hu » Sat Oct 30, 2010 4:28 pm

avendesora wrote:If you don't want the objects to be copied/assigned, don't define the methods so that you get a link time error rather than code that silently does not do what you expect it to at all. (Same thing for the dummy copy constructors.)

As an alternative that has the same effect, but may be more clear, he could make that class inherit from boost::noncopyable, which is added via #include <boost/noncopyable.hpp>.

Also, if using C++0x, he could use the support for explicitly deleted functions to prevent calls to those methods from even being compiled, so the errors show up at compile time instead of link time. For example:

Code: Select all

// Requires C++0x
class Foo
{
    int *ptr;
    Foo(const Foo&) = delete;
    const Foo& operator=(const Foo&) = delete;
public:
    Foo(int a) : ptr(new int) { *ptr = a; }
};

void f()
{
	Foo a(1);
	Foo b(a);	// BAD!
	Foo c = a;	// Also BAD!
}

DaggyStyle · Post by **DaggyStyle** » Sat Oct 30, 2010 6:25 pm

avendesora wrote:
Code: Select all
const bool operator==(const Continent& other);
This is not the correct signature. The 'const' goes at the end. (Same thing for a lot of classes.)

You're declaring and defining a bunch of operator= methods that don't do any assignment.
If you don't want the objects to be copied/assigned, don't define the methods so that you get a link time error rather than code that silently does not do what you expect it to at all. (Same thing for the dummy copy constructors.)

will use the delete option hu has given.
I've wanted to make sure no one will be able to call them.

avendesora wrote: In the MemoryCell thingy, you have this type of construct:
Code: Select all
return (1==(vector[index]->bits | 4));
I'm pretty sure this will always be false. (whatever|4 >= 4.)
To test a bit: (whatever & (1<<bitnumber)) != 0.

I need to check that out, not sure you're right thought.

avendesora wrote:
Code: Select all
const boost::uint8_t& pSkillFitness(const staffPtr obj);
and all the others: C-style pointer casts == bad. That whole file is a mess. Why don't you do something like an enum for the various skills (one for each 'type' of skill), and a single accessor? You should not need those casts at all.

maybe I need to do that, don't remember why I chose that path.

avendesora wrote:

Code: Select all

const std::string Utils::General::getRep(const std::map< std::pair< boost::uint8_t, boost::uint8_t >, std::string >& strDataMap, const boost::uint8_t& num) {
  std::string ans("Null");
  if (num<252) {
    bool stop=false;
    for (std::map< std::pair< boost::uint8_t, boost::uint8_t >, std::string >::const_iterator it=strDataMap.begin();strDataMap.end()!=it and !stop;it++) {
      std::pair< const std::pair< boost::uint8_t, boost::uint8_t >, std::string > item=*it;
      if ((item.first.first<=num) and (item.first.second>=num)) {
    ans=item.second;
    stop=true;
      }
    }
  }
  return ans;
}

Ahh my head hurts

typedef that bloody map type and use the auto keyword.

Code: Select all

const std::string getRep(const supermap& dataMap, const uint8_t& num) {
 if (num < 252) {
  for (auto it = dataMap.begin(); it != dataMap.end(); ++it)
   if (((*it).first.first <= num) &&  ((*it).first.second >= num)))
    return (*it).second;
 }
 return <default>
}

typedef is a good idea, auto maybe as I'm not that familiar with it.
multiple returns? nope.

avendesora wrote: BTW, why take a const uint8_t& as an argument? Why return a const string? (That method could be const though.)

the uint8_t should be a value, it finds the range where the value is located and returns the assigned label of that range.

avendesora wrote: You still have non-void code that doesn't return anything.

in case of non assignment ops and copy ctors, where?

avendesora wrote: Still getting bzip2_error exception when clicking on new game.

strange, maybe the file is corrupted. can you check the logs for something strange?

DaggyStyle · Post by **DaggyStyle** » Sat Oct 30, 2010 6:27 pm

Hu wrote:
avendesora wrote:If you don't want the objects to be copied/assigned, don't define the methods so that you get a link time error rather than code that silently does not do what you expect it to at all. (Same thing for the dummy copy constructors.)
As an alternative that has the same effect, but may be more clear, he could make that class inherit from boost::noncopyable, which is added via #include <boost/noncopyable.hpp>.

Also, if using C++0x, he could use the support for explicitly deleted functions to prevent calls to those methods from even being compiled, so the errors show up at compile time instead of link time. For example:
Code: Select all
// Requires C++0x
class Foo
{
    int *ptr;
    Foo(const Foo&) = delete;
    const Foo& operator=(const Foo&) = delete;
public:
    Foo(int a) : ptr(new int) { *ptr = a; }
};

void f()
{
	Foo a(1);
	Foo b(a);	// BAD!
	Foo c = a;	// Also BAD!
}

excellent idea

Post by Hu » Sat Oct 30, 2010 9:52 pm

DaggyStyle wrote:typedef is a good idea, auto maybe as I'm not that familiar with it.
multiple returns? nope.

In the context that avendesora used it, auto is a C++0x keyword that causes the compiler to compute an appropriate type for storing whatever expression is being saved and then use the computed type. It is very useful in contexts where you are given an iterator from a container and do not or cannot use the full type name of the iterator.

What do you mean "multiple returns? nope."? Do you mean that you dislike having more than one return statement? I have seen programmers with this preference, particularly in C. However, in C++, there is rarely a good reason to restrict yourself arbitrarily to one return statement. Doing so can also lead to very messy code as you try to exit out of the middle without a goto or a return.

DaggyStyle wrote:the uint8_t should be a value, it finds the range where the value is located and returns the assigned label of that range.

I believe the point of the question was to ask why you are using a const uint8_t& instead of just passing the uint8_t by value.

DaggyStyle wrote:
avendesora wrote:You still have non-void code that doesn't return anything.
in case of non assignment ops and copy ctors, where?

The compiler should be able to point you to every offending instance. For C, use -Wreturn-type to get this warning, if it is not already enabled. According to info gcc, the C++ compiler should always warn about returning without a value, except in certain special cases, none of which likely apply here.

DaggyStyle · Post by **DaggyStyle** » Sun Oct 31, 2010 8:35 am

Hu wrote:
DaggyStyle wrote:typedef is a good idea, auto maybe as I'm not that familiar with it.
multiple returns? nope.
In the context that avendesora used it, auto is a C++0x keyword that causes the compiler to compute an appropriate type for storing whatever expression is being saved and then use the computed type. It is very useful in contexts where you are given an iterator from a container and do not or cannot use the full type name of the iterator.

it is time I'd really digg into it.

Hu wrote: What do you mean "multiple returns? nope."? Do you mean that you dislike having more than one return statement? I have seen programmers with this preference, particularly in C. However, in C++, there is rarely a good reason to restrict yourself arbitrarily to one return statement. Doing so can also lead to very messy code as you try to exit out of the middle without a goto or a return.

yes, I don't like multiple returns, I use it only in rare conditions.
also, I don't like breaking while loops.
I like graceful exits.

Hu wrote:
DaggyStyle wrote:the uint8_t should be a value, it finds the range where the value is located and returns the assigned label of that range.
I believe the point of the question was to ask why you are using a const uint8_t& instead of just passing the uint8_t by value.

right, relics of old code, will convert it.

Hu wrote:
DaggyStyle wrote:
avendesora wrote:You still have non-void code that doesn't return anything.
in case of non assignment ops and copy ctors, where?
The compiler should be able to point you to every offending instance. For C, use -Wreturn-type to get this warning, if it is not already enabled. According to info gcc, the C++ compiler should always warn about returning without a value, except in certain special cases, none of which likely apply here.

will look into it.

avendesora · Post by **avendesora** » Sun Oct 31, 2010 10:40 am

Ok, so the immediate crash with bzip2 error was silly: the config.xml has "data" as the root directory. Changing that to "data/" made it go. (That your app crashes in such circumstances is not very nice.)

The crash with the import button on the training thingy is not directly related to that dialog as far as I can tell. It crashes hard in a retranslateUi call (null pointer deref for mUi->mSquadTrainingSheet->horizontalHeaderItem(0) ).
The root of that is most likely mUi->mSquadTrainingSheet->clear() in your main screen setCurrentIndex. This clears everything, including the headers, and the (generated) ui_MainScreen.h is not expecting that at all. Theres a clearContents slot that might be what you were looking for, but I don't understand why you're clearing it, so ymmv.

DaggyStyle wrote:yes, I don't like multiple returns, I use it only in rare conditions.
also, I don't like breaking while loops.
I like graceful exits.

Make sure you app actually quits when you press Alt+F4 then

Excellent points made by Hu as always. I had forgotten about the =delete option which is real nice.

DaggyStyle · Post by **DaggyStyle** » Sun Oct 31, 2010 11:42 am

avendesora wrote:Ok, so the immediate crash with bzip2 error was silly: the config.xml has "data" as the root directory. Changing that to "data/" made it go. (That your app crashes in such circumstances is not very nice.)

that is why this is still an alpha stage project although I'm not quite sure why it is working on my comp and crashed at your. the problem is with the code or the xml?

avendesora wrote: The crash with the import button on the training thingy is not directly related to that dialog as far as I can tell. It crashes hard in a retranslateUi call (null pointer deref for mUi->mSquadTrainingSheet->horizontalHeaderItem(0) ).
The root of that is most likely mUi->mSquadTrainingSheet->clear() in your main screen setCurrentIndex. This clears everything, including the headers, and the (generated) ui_MainScreen.h is not expecting that at all. Theres a clearContents slot that might be what you were looking for, but I don't understand why you're clearing it, so ymmv.

I've inserted the clear command because when I switch to another tab and return to that same tab, the content of the tabs are deleted but the tabs still persists, got any idea how I can solve this issue?

avendesora wrote:
DaggyStyle wrote:yes, I don't like multiple returns, I use it only in rare conditions.
also, I don't like breaking while loops.
I like graceful exits.
Make sure you app actually quits when you press Alt+F4 then

of course, I haven't got to the Alt+F4 part, like said before that is why the project is at alpha stage.

avendesora wrote: Excellent points made by Hu as always. I had forgotten about the =delete option which is real nice.

[/quote]
the dude knows its c++

. I wished I'd knew half of what he knows.

avendesora · Post by **avendesora** » Sun Oct 31, 2010 3:24 pm

DaggyStyle wrote:
avendesora wrote:Ok, so the immediate crash with bzip2 error was silly: the config.xml has "data" as the root directory. Changing that to "data/" made it go. (That your app crashes in such circumstances is not very nice.)
that is why this is still an alpha stage project although I'm not quite sure why it is working on my comp and crashed at your. the problem is with the code or the xml?

The problem is that the code does something like open(config.get("item")+"database/whatever") which gives datadatabase/whatever as a file name if the config has "data", which doesn't exist. That failure is not checked in that code path.

I don't really understand what you mean about the tab things. Switching tabs does not alter their contents, unless you have something weird going on (like having a widget present on multiple tabs, but I don't think that's even possible).
But the way you handle those trees/tables (if I understand it at all) is strange. Unless you have reasons (memory/performance) not to populate the widgets up front, doing that on tab switching is not the proper way to handle things. You should be using table/tree models (I think that's the name) for the data part.
But that's probably not going to be easy because your data structures are really strange.

Code: Select all

bool populateTree=!mUi->mFirstSquadDisp->topLevelItemCount();
uint end=mWorld->activeManagers[0].teamLimit.second+1,start=mWorld->activeManagers[0].teamLimit.first+1;
for (int i=start;(end>i) and populateTree;i++) 
 Utils::GuiUtils::insertPlayerToTeamWidget(mUi->mReserveSquadDisp,mWorld->activeManagers[0].NatHeaderVect,mWorld->activeManagers[0].teamStatusVect->at(i));

Is teamLimit storing indexes into teamStatusVect where different groups start/end? Why not use one vector for each group? Or make teamLimit a vector { 0, teamLimit.first, teamLimit.second, playerCount }. That way you can calculate start/end the same way for all the cases (teamLimit[type]..teamLimit[type+1]). And with not much more you can handle cases 0, 1 and 2 with the same code and not introduce a bug in the last one (see below). But this approach rather than 3 vectors is going to byte you in mysterious ways once you start inserting/deleting stuff in that vector. Keeping the "pointers" in teamLimits up to date (and making sure they stay up to date) is non-trivial.

IMO, multiple assignments on one line can be ok if they are short, readable, and properly spaced. That second line above is a mouthful.
That last line is hell to grok.

There probably is a bug in the same for statement for the Youth part, but the lines are so long it's actually hard to see.

Code: Select all

bool pop = ...;
...;
for (...; something and pop>1; ...)

And pop is testing the same widget in all 3 cases. Is that intended?

Side note: I'm having a hard time understanding your conditionals.

Code: Select all

if ((-1<index) and (8>index))

Any specific reason you write it that way? (Same pretty much all over your code.)
I would have written:

Code: Select all

if ((index > -1) && (index < 8))

or maybe

Code: Select all

if ((-1 < index) && (index < 8))

to make the interval apparent.

DaggyStyle · Post by **DaggyStyle** » Mon Nov 01, 2010 8:46 am

avendesora wrote:
DaggyStyle wrote:
avendesora wrote:Ok, so the immediate crash with bzip2 error was silly: the config.xml has "data" as the root directory. Changing that to "data/" made it go. (That your app crashes in such circumstances is not very nice.)
that is why this is still an alpha stage project although I'm not quite sure why it is working on my comp and crashed at your. the problem is with the code or the xml?
The problem is that the code does something like open(config.get("item")+"database/whatever") which gives datadatabase/whatever as a file name if the config has "data", which doesn't exist. That failure is not checked in that code path.

strange, will try and check that out too.

avendesora wrote: I don't really understand what you mean about the tab things. Switching tabs does not alter their contents, unless you have something weird going on (like having a widget present on multiple tabs, but I don't think that's even possible).
But the way you handle those trees/tables (if I understand it at all) is strange. Unless you have reasons (memory/performance) not to populate the widgets up front, doing that on tab switching is not the proper way to handle things. You should be using table/tree models (I think that's the name) for the data part.

try it, comment the clear command and see what happens.

avendesora wrote: But that's probably not going to be easy because your data structures are really strange.
Code: Select all
bool populateTree=!mUi->mFirstSquadDisp->topLevelItemCount();
uint end=mWorld->activeManagers[0].teamLimit.second+1,start=mWorld->activeManagers[0].teamLimit.first+1;
for (int i=start;(end>i) and populateTree;i++) 
 Utils::GuiUtils::insertPlayerToTeamWidget(mUi->mReserveSquadDisp,mWorld->activeManagers[0].NatHeaderVect,mWorld->activeManagers[0].teamStatusVect->at(i));
Is teamLimit storing indexes into teamStatusVect where different groups start/end? Why not use one vector for each group? Or make teamLimit a vector { 0, teamLimit.first, teamLimit.second, playerCount }. That way you can calculate start/end the same way for all the cases (teamLimit[type]..teamLimit[type+1]). And with not much more you can handle cases 0, 1 and 2 with the same code and not introduce a bug in the last one (see below). But this approach rather than 3 vectors is going to byte you in mysterious ways once you start inserting/deleting stuff in that vector. Keeping the "pointers" in teamLimits up to date (and making sure they stay up to date) is non-trivial.

actually, the ds isn't that strange, I take one vector and populate it with records, in addition, I have two buffer indexes for the boundaries.
because insertion and deletions happens in specific cases, it shouldn't be problematic, also, currently there is no planing to sort the vector or enable custom viewing.

avendesora wrote: IMO, multiple assignments on one line can be ok if they are short, readable, and properly spaced. That second line above is a mouthful.
That last line is hell to grok.

you are right.

avendesora wrote: There probably is a bug in the same for statement for the Youth part, but the lines are so long it's actually hard to see.
Code: Select all
bool pop = ...;
...;
for (...; something and pop>1; ...)
And pop is testing the same widget in all 3 cases. Is that intended?

where exactly?

avendesora wrote: Side note: I'm having a hard time understanding your conditionals.
Code: Select all
if ((-1<index) and (8>index))
Any specific reason you write it that way? (Same pretty much all over your code.)
I would have written:
Code: Select all
if ((index > -1) && (index < 8))
or maybe
Code: Select all
if ((-1 < index) && (index < 8))
to make the interval apparent.

again, where? I need to see the code.

avendesora · Post by **avendesora** » Mon Nov 01, 2010 1:31 pm

DaggyStyle wrote:try it, comment the clear command and see what happens.

If I remove the part with both clears at the top of MainScreen::setCurrentIndex, and protect the populating of the squad training sheet and training schedules tree with checks similar to the other ones (i.e. only fill if they are empty), the thing behaves exactly as I would expect it (and the dialog works). Obviously this is too simple (since they'll never get updated), but I don't see issues switching tabs.

I'm saying the ds are "strange" because you're forced to write code as in that setCurrentIndex method (nearly repetitive, but not quite enough to loop through, and the delta between the 3 cases is tricky). The compiler should be the one doing the boring repetitive stuff, not you

DaggyStyle wrote:
avendesora wrote:There probably is a bug in the same for statement for the Youth part, but the lines are so long it's actually hard to see.
Code: Select all
bool pop = ...;
...;
for (...; something and pop>1; ...)
And pop is testing the same widget in all 3 cases. Is that intended?
where exactly?

MainScreen.cpp lines 480, 488 and 496 for the check, line 498 for the probable bug.

DaggyStyle wrote:
avendesora wrote:Side note: I'm having a hard time understanding your conditionals.
Code: Select all
if ((-1<index) and (8>index))
again, where? I need to see the code.

All the conditionals in MainScreen::setCurrentIndex (including the ones in the for loops), same file line 375 for example. No problem with the code, I just find it backwards - just a style issue.

DaggyStyle · Post by **DaggyStyle** » Mon Nov 01, 2010 1:52 pm

avendesora wrote:
DaggyStyle wrote:try it, comment the clear command and see what happens.
If I remove the part with both clears at the top of MainScreen::setCurrentIndex, and protect the populating of the squad training sheet and training schedules tree with checks similar to the other ones (i.e. only fill if they are empty), the thing behaves exactly as I would expect it (and the dialog works). Obviously this is too simple (since they'll never get updated), but I don't see issues switching tabs.

I'm saying the ds are "strange" because you're forced to write code as in that setCurrentIndex method (nearly repetitive, but not quite enough to loop through, and the delta between the 3 cases is tricky). The compiler should be the one doing the boring repetitive stuff, not you
DaggyStyle wrote:
avendesora wrote:There probably is a bug in the same for statement for the Youth part, but the lines are so long it's actually hard to see.
Code: Select all
bool pop = ...;
...;
for (...; something and pop>1; ...)
And pop is testing the same widget in all 3 cases. Is that intended?
where exactly?
MainScreen.cpp lines 480, 488 and 496 for the check, line 498 for the probable bug.
DaggyStyle wrote:
avendesora wrote:Side note: I'm having a hard time understanding your conditionals.
Code: Select all
if ((-1<index) and (8>index))
again, where? I need to see the code.
All the conditionals in MainScreen::setCurrentIndex (including the ones in the for loops), same file line 375 for example. No problem with the code, I just find it backwards - just a style issue.

I really have not idea why it doesn't work fine here, what qt version are you using?

avendesora · Post by **avendesora** » Mon Nov 01, 2010 1:58 pm

Code: Select all

 ~ $ equery l qt-
[ Searching for package 'qt-' in all categories among: ]
 * installed packages
[I--] [  ] dev-util/qt-creator-1.3.1-r1 (0)
[I--] [ ~] x11-libs/qt-assistant-4.6.3 (4)
[I--] [ ~] x11-libs/qt-core-4.6.3 (4)
[I--] [ ~] x11-libs/qt-dbus-4.6.3 (4)
[I--] [ ~] x11-libs/qt-gui-4.6.3 (4)
[I--] [ ~] x11-libs/qt-opengl-4.6.3 (4)
[I--] [ ~] x11-libs/qt-qt3support-4.6.3 (4)
[I--] [ ~] x11-libs/qt-script-4.6.3 (4)
[I--] [ ~] x11-libs/qt-sql-4.6.3-r2 (4)
[I--] [ ~] x11-libs/qt-svg-4.6.3 (4)
[I--] [ ~] x11-libs/qt-test-4.6.3 (4)
[I--] [ ~] x11-libs/qt-webkit-4.6.3-r1 (4)
[I--] [ ~] x11-libs/qt-xmlpatterns-4.6.3 (4)

 ~ $ gcc-config -l
 [1] mingw32-4.4.4
 [2] mingw32-4.5.1 *

 [3] x86_64-pc-linux-gnu-4.3.4
 [4] x86_64-pc-linux-gnu-4.4.4 *

 ~ $ eselect boost list
Available boost versions:
  [1]   boost-1.35/default
  [2]   boost-1.41/default *

Post by Hu » Tue Nov 02, 2010 2:56 am

Rather than storing indexes, it may be more useful to store iterators from the associated container. This has two advantages. First, it allows you to detect erroneous assignment of unrelated integers into the container. Second, it allows you to enable _GLIBCXX_DEBUG to get detailed checking that the iterators are used correctly. Among the things detected by that directive:

Attempt to dereference an off-the-end iterator.
Attempt to increment an iterator that is already equal to end().
Attempt to dereference a singular iterator.

The only major downside is that storing iterators does mean that you must be careful about recomputing them after operations which you know invalidate the saved iterators.

DaggyStyle · Post by **DaggyStyle** » Tue Nov 02, 2010 10:42 pm

avendesora, thanks, the error is solved, you were right

Hu wrote:Rather than storing indexes, it may be more useful to store iterators from the associated container. This has two advantages. First, it allows you to detect erroneous assignment of unrelated integers into the container. Second, it allows you to enable _GLIBCXX_DEBUG to get detailed checking that the iterators are used correctly. Among the things detected by that directive:
Attempt to dereference an off-the-end iterator.

Attempt to increment an iterator that is already equal to end().

Attempt to dereference a singular iterator.
The only major downside is that storing iterators does mean that you must be careful about recomputing them after operations which you know invalidate the saved iterators.

ok, does the iterators behave like list?

Post by Hu » Wed Nov 03, 2010 2:20 am

DaggyStyle wrote:ok, does the iterators behave like list?

Iterators behave mostly like pointers. You can increment them, compare them, and dereference them to get a reference to the underlying data. For bidirectional iterators, you can decrement them. For random access iterators, you can add or subtract distances to them. Some very old attempts at STL, such as that shipped with Microsoft Visual Studio 6, actually used pointers via a typedef. Fortunately, modern implementations of STL both on gcc and Microsoft Visual Studio now use proper classes for their iterators.