> On März 1, 2016, 8:16 nachm., Christian David wrote: > > libkgpgfile/kgpgfile.cpp, line 162 > > <https://git.reviewboard.kde.org/r/127108/diff/2/?file=447716#file447716line162> > > > > I removed this test because ```QFile::open()``` will check that anyway > > and more important: This can be wrong under some circumstances.
Not sure. Please check against https://bugs.kde.org/show_bug.cgi?id=256750 for which this has been added. > On März 1, 2016, 8:16 nachm., Christian David wrote: > > kmymoney/views/kmymoneyview.cpp, line 1185 > > <https://git.reviewboard.kde.org/r/127108/diff/2/?file=447715#file447715line1185> > > > > In this line I get a warning which I do not understand: > > > > kmymoney/views/kmymoneyview.cpp: In constructor > > ‘KMyMoneyView::saveToLocalFile(const QString&, IMyMoneyStorageFormat*, > > bool, const > > QString&)::restorePreviousSettingsHelper::restorePreviousSettingsHelper(mode_t)’: > > kmymoney/views/kmymoneyview.cpp:1176:22: warning: narrowing > > conversion of ‘umask(((~ mode) & 511u))’ from ‘__mode_t {aka unsigned int}’ > > to ‘int’ inside { } [-Wnarrowing] > > m_oldMask{umask((~mode) & static_cast<mode_t>(0777))} You convert an 'unsigned int' (the type of the result of your expression) to an 'int' (the type of m_oldMask) and loose one bit of information (since the int needs one bit over the unsigned for its sign bit). This does not hurt here, as only 9 bits are relevant. You can try if m_oldMask { static_cast<int>((~mode) & 0777u) } or make m_oldMask of type mode_t or variations thereof. - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127108/#review93030 ----------------------------------------------------------- On März 1, 2016, 8:03 nachm., Christian David wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127108/ > ----------------------------------------------------------- > > (Updated März 1, 2016, 8:03 nachm.) > > > Review request for KMymoney. > > > Bugs: 356399 > http://bugs.kde.org/show_bug.cgi?id=356399 > > > Repository: kmymoney > > > Description > ------- > > Fixed potential memory leak during saving > > A pointer was not deleted before throwing exceptions in > KMyMoneyView::saveFile. Also renamed the pointer. > > > Changed way of saving files which should fix some bugs > > The save operation seems to fail every time - it never changed the > orginal file and never reported any issues. I did not find the exact > reason for this bug but I am quite sure it was caused by an incorret > usage of QSaveFile (under some circumstances close() instead of commit() > was called). > > Now KMyMoney creates its own temporary file to write to (if needed). > This also works using KGpgFile, which should fix Bug 356399. > > The remove() and rename() operations are not atomic which is not so > good as this could result in dataloss if the first operation fails. > However, this is the best OS independet process I could find. > > Errors during writing of compressed files may not be detected. I think > this issue should be fixed upstream. > > BUG: 356399 > FIXED-IN: 5.0 > REVIEW: 127108 > > Fixed bug while saving GPG encrypted files > > A call to QSaveFile::commit() was missing. So QSaveFile always assumed > an error and discarded the changes. > > Used this fix for minor clean-ups. > > > Diffs > ----- > > CMakeLists.txt a82d70eed87c5f8a8052b969c0a8a17d82ef8b1d > kmymoney/views/kmymoneyview.h c4a769c2bf88083ea56283d683d7f0f7f0466875 > kmymoney/views/kmymoneyview.cpp 284bd6a2657982c25790b2428730f279fc86504c > libkgpgfile/kgpgfile.cpp b1870be92edb833ed30f369e3e0ca0f320fe147b > > Diff: https://git.reviewboard.kde.org/r/127108/diff/ > > > Testing > ------- > > I saved a file several times using the compressed, uncompressed and anonymous > format. I could not test the GPG part because none of my keys is currently > shown by the save dialog. > > New: I tested it with GPG encrypted files. > > > Thanks, > > Christian David > >
