View Issue Details

IDProjectCategoryView StatusLast Update
0030403CommunityOCCT:Foundation Classespublic2019-07-20 11:55
ReporterBenjaminBihler Assigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformMingw-w64OSWindows 
Product Version7.3.0 
Target Version7.4.0Fixed in Version7.4.0 
Summary0030403: Foundation Classes - Overwriting Big "BinOcaf" Files Does Not Reduce Their Size
DescriptionIf I have a big document file here and I overwrite it with a small document file, then everything works fine except that the file size isn't reduced. If I store it to a new file, the file size is as small as expected.

The solution is to add "std::ios::trunc" to the mode of the line

OSD_OpenStream (aFileStream, theFileName, std::ios::out | std::ios::binary);

which is here line 74 of BinLDrivers_DocumentStorageDriver.cxx. I guess that it would be the best to add "std::ios::trunc" to all calls to OSD_OpenStream where the mode contains the bit "std::ios::out".

Note: I use Mingw-w64 here, but I guess that this doesn't cause a difference.
Steps To ReproduceNot done yet
TagsNo tags attached.
Test case numberNot required

Relationships

child of 0029081 closedabv Foundation Classes, OSD_OpenStream - handle UNICODE file paths specifically in case of Mingw-w64 

Activities

BenjaminBihler

2019-07-11 10:25

developer   ~0085588

I have been disappointed to see that the target version has been set to 7.5.0. This issue should be quick and easy to fix.

According to https://en.cppreference.com/w/cpp/io/ios_base/openmode, "std::ios::trunc" discards the contents of the stream when opening, so there shouldn't be any drawback when overwriting files.

Would you reconsider this issue, if I commited a proposal for the fix? Or do you have a reason to expect the issue to be more complex?

mpv

2019-07-11 10:37

developer   ~0085590

Proposal for the fix would be appreciated. Fix of this issue also supposes non-regression test creation, which is quite expensive for such a minor issue.

git

2019-07-11 12:18

administrator   ~0085592

Branch CR30403 has been created by BenjaminBihler.

SHA-1: 1d747136ea931d15e173eb3668b16b1361da1136


Detailed log of new commits:

Author: Benjamin Bihler
Date: Thu Jul 11 11:16:12 2019 +0200

    0030403: Overwriting Big "BinOcaf" Files Does Not Reduce Their Size
    
    Added std::ios::trunc to all file open modes used for writing.

BenjaminBihler

2019-07-11 12:24

developer   ~0085593

While implementing and checking the fix, I have realized that this is even more than a minor issue. XmlLDrivers_DocumentStorageDriver and VrmlAPI_Writer have been writing text files without truncating the output stream. In this case when overwriting large files, parts of the old files were still there, leading most probably to wrong output files (the remainders of old large files have been seemingly ignored with BinOcaf files).

Since I am not really familiar with writing test scripts, I won't do that, sorry.

mpv

2019-07-16 17:27

developer   ~0085674

  Hello Benjamin,

I have analysed your fix and could not reproduce the problem.

What I did:

- I saved BinOcaf documents into the same file. First big, then smaller into the same file. The content and size of file correctly reduced on Windows Visual Studio and Linux gcc platforms.

- I downloaded MinGW-w64 from here: https://sourceforge.net/projects/mingw-w64/
Since I have no OCCT compiled with it, I created a simplified program to reproduce the same behavior as in BinLDrivers_DocumentStorageDriver::Write

#include <fstream>
using namespace std;
int main() {
  ofstream aFileStream("a.out", std::ios::out | std::ios::binary);
  aFileStream<<"Hello world!";
  aFileStream.close();
  ofstream aFileStream2("a.out", std::ios::out | std::ios::binary);
  aFileStream2<<"Bye!";
  aFileStream2.close();
}

After compilation is with MinGW, I found that result in "a.out" is "Bye!", size 4 bytes. Behavior does not change after addition of "std::ios::trunc".

- I found, for an example here:
http://stdcxx.apache.org/doc/stdlibug/30-3.html
that "For output file streams the open mode out is equivalent to out|trunc, that is, you can omit the trunc flag."

After this investigation it looks for me like your patch is useless.

So,

1. Could you check that my simple program works correctly/incorrectly on your environment
2. Could you describe exactly (for an example, in a small sequence of C++ commands) how you may reproduce this wrong behavior using OCCT BinOcaf document(s).

BenjaminBihler

2019-07-17 11:35

developer   ~0085694

You are absolutely right. See also https://stackoverflow.com/questions/57069930/why-does-stdofstream-truncate-without-stdios-basetrunc. The misbehaviour here has been introduced by a small bug in special workaround code for Mingw-w64. Since I use Mingw-w64, I have suffered from that. My former fix has solved the problem, but with better understanding a more elegant fix is possible...

git

2019-07-17 11:35

administrator   ~0085695

Branch CR30403 has been updated by BenjaminBihler.

SHA-1: c1dc8f9ea723b9b8e7b0c8bcfeb655c1e43a2fed


Detailed log of new commits:

Author: Benjamin Bihler
Date: Wed Jul 17 10:30:13 2019 +0200

    0030403: Application Framework - Overwriting Big BinOcaf Files Does Not Reduce Their Size
    
    Added truncating flag to fopen() flags for ios_base::out, since truncating
    is implied by C++ output flag.

Author: Benjamin Bihler
Date: Wed Jul 17 10:10:07 2019 +0200

    0030403: Overwriting Big "BinOcaf" Files Does Not Reduce Their Size
    
    This reverts commit 1d747136ea931d15e173eb3668b16b1361da1136.

mpv

2019-07-17 12:01

developer   ~0085698

So, it is not Application Framework problem, but a consequence of 29081 fix.

I may add that the currently proposed solution is not optimal:

aFlags |= O_TRUNC;

may called twice, in few lines of code.

BenjaminBihler

2019-07-17 12:10

developer   ~0085699

You are again right.

git

2019-07-17 12:12

administrator   ~0085700

Branch CR30403 has been updated by BenjaminBihler.

SHA-1: 71433cd3de0d24dd198cb1e69a41f7b131743336


Detailed log of new commits:

Author: Benjamin Bihler
Date: Wed Jul 17 11:10:31 2019 +0200

    0030403: Foundation Classes - Overwriting Big BinOcaf Files Does Not Reduce Their Size
    
    Removed unnecessary conditional adding of the truncation flag.

git

2019-07-17 12:37

administrator   ~0085703

Branch CR30403_1 has been created by kgv.

SHA-1: 0b915470254e2bac9fe749d0129727377198ddc1


Detailed log of new commits:

Author: Benjamin Bihler
Date: Thu Jul 11 11:16:12 2019 +0200

    0030403: Application Framework - Overwriting Big BinOcaf Files Does Not Reduce Their Size
    
    OSD_OpenFileDescriptor(), added truncating flag to fopen() flags for std::ios_base::out,
    since truncating is implied by C++ output flag.
    This fixes misbehavior of OSD_OpenStream() on MinGW-w64 platform.

kgv

2019-07-17 12:38

developer   ~0085704

Please raise the patch.

bugmaster

2019-07-18 10:09

administrator   ~0085733

Tested in framework of WEEK-29 branch

Combination -
OCCT branch : WEEK-29
master SHA - 00da65729fc6ff2741349508c0dc5480542e0e64
32ce09545dc9c46a47a51a964a24b1f472e6c2c4
Products branch : WEEK-29 SHA - ce8f14216a548db739b749891b6f800b0d5c6e8b
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 16228.200000000006 / 16237.640000000089 [-0.06%]
Products
Total CPU difference: 10488.620000000039 / 10517.890000000038 [-0.28%]
Windows-64-VC14:
OCCT
Total CPU difference: 17628.96875 / 17623.078125 [+0.03%]
Products
Total CPU difference: 12067.953125 / 12058.53125 [+0.08%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2019-07-20 11:55

administrator   ~0085835

Branch CR30403 has been deleted by inv.

SHA-1: 71433cd3de0d24dd198cb1e69a41f7b131743336

git

2019-07-20 11:55

administrator   ~0085836

Branch CR30403_1 has been deleted by inv.

SHA-1: 0b915470254e2bac9fe749d0129727377198ddc1

Related Changesets

occt: master 718d07fe

2019-07-11 09:16:12

BenjaminBihler


Committer: bugmaster Details Diff
0030403: Application Framework - Overwriting Big BinOcaf Files Does Not Reduce Their Size

OSD_OpenFileDescriptor(), added truncating flag to fopen() flags for std::ios_base::out,
since truncating is implied by C++ output flag.
This fixes misbehavior of OSD_OpenStream() on MinGW-w64 platform.
Affected Issues
0030403
mod - src/OSD/OSD_OpenFile.cxx Diff File

Issue History

Date Modified Username Field Change
2018-12-10 16:28 BenjaminBihler New Issue
2018-12-10 16:28 BenjaminBihler Assigned To => mpv
2019-07-10 09:01 abv Target Version 7.4.0 => 7.5.0
2019-07-11 10:25 BenjaminBihler Note Added: 0085588
2019-07-11 10:37 mpv Note Added: 0085590
2019-07-11 12:18 git Note Added: 0085592
2019-07-11 12:24 BenjaminBihler Note Added: 0085593
2019-07-11 12:24 BenjaminBihler Status new => resolved
2019-07-11 12:24 BenjaminBihler Steps to Reproduce Updated
2019-07-11 14:34 kgv Summary Overwriting Big "BinOcaf" Files Does Not Reduce Their Size => Application Framework - Overwriting Big "BinOcaf" Files Does Not Reduce Their Size
2019-07-12 17:50 abv Target Version 7.5.0 => 7.4.0
2019-07-16 17:27 mpv Note Added: 0085674
2019-07-16 17:27 mpv Assigned To mpv => BenjaminBihler
2019-07-16 17:27 mpv Status resolved => feedback
2019-07-17 11:32 BenjaminBihler Relationship added related to 0029081
2019-07-17 11:35 BenjaminBihler Note Added: 0085694
2019-07-17 11:35 git Note Added: 0085695
2019-07-17 11:36 BenjaminBihler Assigned To BenjaminBihler => mpv
2019-07-17 11:36 BenjaminBihler Status feedback => resolved
2019-07-17 12:01 mpv Note Added: 0085698
2019-07-17 12:01 mpv Assigned To mpv => kgv
2019-07-17 12:01 mpv Category OCCT:Application Framework => OCCT:Foundation Classes
2019-07-17 12:01 mpv Summary Application Framework - Overwriting Big "BinOcaf" Files Does Not Reduce Their Size => Foundation Classes - Overwriting Big "BinOcaf" Files Does Not Reduce Their Size
2019-07-17 12:10 BenjaminBihler Note Added: 0085699
2019-07-17 12:12 git Note Added: 0085700
2019-07-17 12:37 git Note Added: 0085703
2019-07-17 12:38 kgv Note Added: 0085704
2019-07-17 12:38 kgv Assigned To kgv => bugmaster
2019-07-17 12:38 kgv Status resolved => reviewed
2019-07-17 12:38 kgv Relationship replaced child of 0029081
2019-07-18 10:08 bugmaster Test case number => Not required
2019-07-18 10:09 bugmaster Note Added: 0085733
2019-07-18 10:09 bugmaster Status reviewed => tested
2019-07-20 11:42 bugmaster Changeset attached => occt master 718d07fe
2019-07-20 11:42 bugmaster Status tested => verified
2019-07-20 11:42 bugmaster Resolution open => fixed
2019-07-20 11:55 git Note Added: 0085835
2019-07-20 11:55 git Note Added: 0085836