Review Request 127108: Changed way of saving files which should fix some bugs

[Thomas Baumgart]

> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20160304/e9c60ee8/attachment-0001.html>