View Issue Details

IDProjectCategoryView StatusLast Update
0029344Open CASCADEOCCT:Foundation Classespublic2020-12-02 13:02
Reporterkgv Assigned Tobugmaster  
PrioritynormalSeverityintegration request 
Status closedResolutionfixed 
Target Version7.3.0Fixed in Version7.3.0 
Summary0029344: Foundation Classes, TCollection_AsciiString - replace confusing strncpy with memcpy
DescriptionTCollection_AsciiString implementation uses strncpy + strlen combination which does not make sense, because after strlen the length of the string is already determined and plain memcpy can be called instead.
Steps To ReproduceN/A
TagsNo tags attached.
Test case numberNot required

Relationships

parent of 0031972 closedbugmaster Community Application Framework, FSD_CmpFile - exception on reading file in old persistence format with Windows EOL 

Activities

git

2017-11-24 18:38

administrator   ~0072470

Branch CR29344 has been created by kgv.

SHA-1: 3f942778d3353229abc97a07f89bd359d752f034


Detailed log of new commits:

Author: kgv
Date: Fri Nov 24 18:37:01 2017 +0300

    0029344: Foundation Classes, TCollection_AsciiString - replace confusing strncpy with memcpy
    
    strncpy usage within TCollection_AsciiString has been replaced by memcpy
    where string lenght has been already determined.
    
    TCollection_AsciiString(const char* , int ) and TCollection_AsciiString::SetValue()
    have been modified to throw exception on attempt to define invalid length of the string.

git

2017-11-24 22:02

administrator   ~0072473

Branch CR29344 has been updated by kgv.

SHA-1: f87d49a5482d60b1e21f5ecd77498cfb5e5d55c6


Detailed log of new commits:

Author: kgv
Date: Fri Nov 24 22:00:08 2017 +0300

    # fix regressions

git

2017-11-24 22:08

administrator   ~0072474

Branch CR29344_1 has been created by kgv.

SHA-1: 4a11238158b7c47ff1a812bba1150acf7c510fa2


Detailed log of new commits:

Author: kgv
Date: Fri Nov 24 18:37:01 2017 +0300

    0029344: Foundation Classes, TCollection_AsciiString - replace confusing strncpy with memcpy
    
    strncpy() usage within TCollection_AsciiString has been replaced by memcpy()
    where string length has been already determined.
    
    TCollection_AsciiString::SetValue() now throws exception
    on attempt to set '\0' - TCollection_AsciiString::Trunc() should be used instead.
    TCollection_AsciiString(const char* , int ) has been modified to properly set string length
    in case of NULL-terminator appeared earlier then specified length.
    
    Interface_LineBuffer has been revised for using NCollection_Array1 instead of TCollection_AsciiString
    to avoid misusing TCollection_AsciiString interface.

kgv

2017-11-24 23:26

developer   ~0072476

Last edited: 2017-11-26 12:49

Patch is ready for review.

http://jenkins-test-10.nnov.opencascade.com/view/CR29344_1-master-KGV

git

2017-11-25 15:28

administrator   ~0072481

Branch CR29344_1 has been updated forcibly by kgv.

SHA-1: e73d921a92dcb5f5b4c30f73d7ed0b61702eb8a7

abv

2017-11-27 15:06

manager   ~0072498

Reviewed, please integrate

abv

2017-11-27 15:07

manager   ~0072499

Actually I have a doubt whether having null symbol in the middle of the string should really be prohibited...

kgv

2017-11-27 15:13

developer   ~0072500

Last edited: 2017-11-27 15:13

> Actually I have a doubt whether having null symbol in the middle
> of the string should really be prohibited..
This is misconception - TCollection_AsciiString is supposed to know the length of the string to behave in deterministic way.
Although one would say that constant string class should not have method SetValue() at all.
Considering an allocation of buffer of greater size then actual string length, another mechanism might be considered - e.g. std::string has two properties: actual length and capacity.

bugmaster

2017-11-27 16:07

administrator   ~0072502

Combination -
OCCT branch : CR29344_1 SHA - e73d921a92dcb5f5b4c30f73d7ed0b61702eb8a7
Products branch : master SHA - e6803066bfddb413c99c934cc8af4386abcbc679
was compiled on Linux, MacOS and Windows platforms and tested on optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian70-64:
OCCT
Total CPU difference: 20052.220000000558 / 19837.270000000357 [+1.08%]
Products
Total CPU difference: 7995.2600000000875 / 8039.350000000084 [-0.55%]
Windows-64-VC10:
OCCT
Total CPU difference: 17873.174970898603 / 17832.614710898575 [+0.23%]
Products
Total CPU difference: 8036.500715699982 / 8019.621407499985 [+0.21%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2017-12-11 12:00

administrator   ~0072894

Branch CR29344 has been deleted by kgv.

SHA-1: f87d49a5482d60b1e21f5ecd77498cfb5e5d55c6

git

2017-12-11 12:00

administrator   ~0072895

Branch CR29344_1 has been deleted by kgv.

SHA-1: e73d921a92dcb5f5b4c30f73d7ed0b61702eb8a7

Related Changesets

occt: master 48a2dd20

2017-11-24 15:37:01

kgv


Committer: bugmaster Details Diff
0029344: Foundation Classes, TCollection_AsciiString - replace confusing strncpy with memcpy

strncpy() usage within TCollection_AsciiString has been replaced by memcpy()
where string length has been already determined.

TCollection_AsciiString::SetValue() now throws exception
on attempt to set '\0' - TCollection_AsciiString::Trunc() should be used instead.
TCollection_AsciiString(const char* , int ) has been modified to properly set string length
in case of NULL-terminator appeared earlier then specified length.

Interface_LineBuffer has been revised for using NCollection_Array1 instead of TCollection_AsciiString
to avoid misusing TCollection_AsciiString interface.
Affected Issues
0029344
mod - src/Interface/Interface_LineBuffer.cxx Diff File
mod - src/Interface/Interface_LineBuffer.hxx Diff File
mod - src/TCollection/TCollection_AsciiString.cxx Diff File

Issue History

Date Modified Username Field Change
2017-11-24 18:34 kgv New Issue
2017-11-24 18:34 kgv Assigned To => abv
2017-11-24 18:38 git Note Added: 0072470
2017-11-24 22:02 git Note Added: 0072473
2017-11-24 22:08 git Note Added: 0072474
2017-11-24 23:26 kgv Note Added: 0072476
2017-11-24 23:26 kgv Status new => resolved
2017-11-25 15:28 git Note Added: 0072481
2017-11-26 12:49 kgv Note Edited: 0072476
2017-11-27 15:06 abv Note Added: 0072498
2017-11-27 15:06 abv Assigned To abv => bugmaster
2017-11-27 15:06 abv Status resolved => reviewed
2017-11-27 15:07 abv Note Added: 0072499
2017-11-27 15:13 kgv Note Added: 0072500
2017-11-27 15:13 kgv Note Edited: 0072500
2017-11-27 16:05 bugmaster Test case number => Not required
2017-11-27 16:07 bugmaster Note Added: 0072502
2017-11-27 16:07 bugmaster Status reviewed => tested
2017-12-08 23:00 bugmaster Changeset attached => occt master 48a2dd20
2017-12-08 23:00 bugmaster Status tested => verified
2017-12-08 23:00 bugmaster Resolution open => fixed
2017-12-11 12:00 git Note Added: 0072894
2017-12-11 12:00 git Note Added: 0072895
2018-02-20 12:58 aiv Target Version 7.4.0 => 7.3.0
2018-06-29 21:15 aiv Fixed in Version => 7.3.0
2018-06-29 21:19 aiv Status verified => closed
2020-12-02 13:02 kgv Relationship added parent of 0031972