MantisBT - Open CASCADE
View Issue Details
0029344Open CASCADE[OCCT] OCCT:Foundation Classespublic2017-11-24 18:342018-06-29 21:19
kgv 
bugmaster 
normalintegration request 
closedfixed 
 
[OCCT] 7.3.0[OCCT] 7.3.0 
Not required
0029344: Foundation Classes, TCollection_AsciiString - replace confusing strncpy with memcpy
TCollection_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.
N/A
No tags attached.
Issue History
2017-11-24 18:34kgvNew Issue
2017-11-24 18:34kgvAssigned To => abv
2017-11-24 18:38gitNote Added: 0072470
2017-11-24 22:02gitNote Added: 0072473
2017-11-24 22:08gitNote Added: 0072474
2017-11-24 23:26kgvNote Added: 0072476
2017-11-24 23:26kgvStatusnew => resolved
2017-11-25 15:28gitNote Added: 0072481
2017-11-26 12:49kgvNote Edited: 0072476bug_revision_view_page.php?bugnote_id=72476#r18229
2017-11-27 15:06abvNote Added: 0072498
2017-11-27 15:06abvAssigned Toabv => bugmaster
2017-11-27 15:06abvStatusresolved => reviewed
2017-11-27 15:07abvNote Added: 0072499
2017-11-27 15:13kgvNote Added: 0072500
2017-11-27 15:13kgvNote Edited: 0072500bug_revision_view_page.php?bugnote_id=72500#r18231
2017-11-27 16:05bugmasterTest case number => Not required
2017-11-27 16:07bugmasterNote Added: 0072502
2017-11-27 16:07bugmasterStatusreviewed => tested
2017-12-08 23:00bugmasterChangeset attached => occt master 48a2dd20
2017-12-08 23:00bugmasterStatustested => verified
2017-12-08 23:00bugmasterResolutionopen => fixed
2017-12-11 12:00gitNote Added: 0072894
2017-12-11 12:00gitNote Added: 0072895
2018-01-29 15:10kgvRelationship addedrelated to 0029460
2018-02-20 12:58aivTarget Version7.4.0 => 7.3.0
2018-06-29 21:15aivFixed in Version => 7.3.0
2018-06-29 21:19aivStatusverified => closed

Notes
(0072470)
git   
2017-11-24 18:38   
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.
(0072473)
git   
2017-11-24 22:02   
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

(0072474)
git   
2017-11-24 22:08   
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.
(0072476)
kgv   
2017-11-24 23:26   
(edited on: 2017-11-26 12:49)
Patch is ready for review.

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

(0072481)
git   
2017-11-25 15:28   
Branch CR29344_1 has been updated forcibly by kgv.

SHA-1: e73d921a92dcb5f5b4c30f73d7ed0b61702eb8a7
(0072498)
abv   
2017-11-27 15:06   
Reviewed, please integrate
(0072499)
abv   
2017-11-27 15:07   
Actually I have a doubt whether having null symbol in the middle of the string should really be prohibited...
(0072500)
kgv   
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.

(0072502)
bugmaster   
2017-11-27 16:07   
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
(0072894)
git   
2017-12-11 12:00   
Branch CR29344 has been deleted by kgv.

SHA-1: f87d49a5482d60b1e21f5ecd77498cfb5e5d55c6
(0072895)
git   
2017-12-11 12:00   
Branch CR29344_1 has been deleted by kgv.

SHA-1: e73d921a92dcb5f5b4c30f73d7ed0b61702eb8a7