View Issue Details

IDProjectCategoryView StatusLast Update
0026179Open CASCADEOCCT:Codingpublic2016-04-20 15:50
Reporterkgv Assigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformMacOSOS 
Target Version7.0.0Fixed in Version7.0.0 
Summary0026179: Coding rules - eliminate -Wdeprecated-declarations CLang warnings on tmpnam() usage
DescriptionThe following warnings should be eliminated:
occt/src/OSD/OSD_Directory.cxx:64:32: warning: 'tmpnam' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of tmpnam(3), it is highly recommended that you use mkstemp(3) instead. [-Wdeprecated-declarations]
Standard_CString        name = tmpnam(NULL);
                               ^
occt/src/OSD/OSD_File.cxx:255:15: warning: 'tmpnam' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of tmpnam(3), it is highly recommended that you use mkstemp(3) instead. [-Wdeprecated-declarations]
 char *name = tmpnam((char*) 0) ;
              ^



gcc 4.9.2 on Linux generates similar warning:
In function "OSD_Directory::BuildTemporary()':
OSD_Directory.cxx:(.text+0x17b): warning: the use of "tmpnam' is dangerous, better use "mkstemp'
Steps To ReproduceN/A
TagsNo tags attached.
Test case numberNot needed

Activities

git

2015-10-30 14:39

administrator   ~0047649

Branch CR26179 has been created by rkv.

SHA-1: 0c803eaa2e15cbf7dc62a73c723df0d5e57b3e40


Detailed log of new commits:

Author: rkv
Date: Fri Oct 30 13:44:03 2015 +0300

    0026179: Coding rules - eliminate -Wdeprecated-declarations CLang warnings on tmpnam() usage

rkv

2015-10-30 14:41

developer   ~0047650

mkstemp() and mkdtemp() are used instead of tempnam().

kgv

2015-10-30 15:11

developer   ~0047657

+char                    name[] = "tmpXXXXXX";

the patch would not work as expected because temporary file will be created in current working directory.

Please check if these methods are used at all and consider removing them.

git

2015-11-09 13:42

administrator   ~0047946

Branch CR26179 has been updated forcibly by rkv.

SHA-1: 20a36de55b9a47cafb33f3accfb93530817517c9

kgv

2015-11-09 17:11

developer   ~0047958

Last edited: 2015-11-09 17:11

-OSD_File OSD_File::BuildTemporary(){
+void OSD_File::BuildTemporary(){

since this function is now method of class - it would be better to close current file within OSD_File instance before opening new one for consistency and safety.

+
+  Standard_EXPORT Standard_Integer ReadLine (TCollection_AsciiString& Buffer, const Standard_Integer NByte);

could you please add description of this method?

+Standard_Integer OSD_File::ReadLine (TCollection_AsciiString& Buffer, const Standard_Integer NByte) {
+  Standard_Integer NbyteRead;
+  ReadLine(Buffer, NByte, NbyteRead);
+  return NbyteRead;

Since this is a small wrapper over existing method - maybe it would be better to inline body into method definition.

git

2015-11-10 09:24

administrator   ~0047971

Branch CR26179 has been updated forcibly by rkv.

SHA-1: acd920e960a5a2d17d21ca9902ef51a67cab082d

kgv

2015-11-10 09:53

developer   ~0047976

> Branch CR26179 has been updated forcibly by rkv.
please do not override the branches already sent to review.
The best practice is to push corrections as dedicated commits (useful to keep history and to revise small corrections) and to prepare new branch with squashed changes (for actual testing and integration).

+  Standard_EXPORT inline Standard_Integer ReadLine (

Standard_EXPORT should not be used within inline methods - please drop the keyword.

  TEST_RAISE(  "Close"  );
 
- CloseHandle (myFileHandle);
+ if (myFileHandle != INVALID_HANDLE_VALUE)
+  CloseHandle (myFileHandle);

there is no much use in this check since TEST_RAISE should throw exception in this case due to weird OSD_File design.

> since this function is now method of class
> - it would be better to close current file
> within OSD_File instance before opening new one for consistency and safety.
I don't see any changes related to this comment - OSD_File::BuildTemporary() still overrides myFileHandle, myFileChannel without any check.
I should at least throw exception as in OSD_File::Open():
> Standard_ProgramError::Raise("OSD_File::Open : file is already open")

git

2015-11-10 12:00

administrator   ~0047987

Branch CR26179 has been updated by rkv.

SHA-1: b96e1c59b456bf87e257b496df7b1899d96abd49


Detailed log of new commits:

Author: rkv
Date: Tue Nov 10 12:00:30 2015 +0300

    0026179: Coding rules - eliminate -Wdeprecated-declarations CLang warnings on tmpnam() usage
    
    Drop Standard_EXPORT for inline method declaration.
    Remove unnecessary check when closing OSD_File.
    Close old file before building a new temporary file.

kgv

2015-11-10 12:05

developer   ~0047988

Please test the patch.

mkv

2015-11-11 19:34

tester   ~0048077

Dear BugMaster,
Branch CR26179 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
SHA-1: b96e1c59b456bf87e257b496df7b1899d96abd49

Number of compiler warnings:

occt component :
Linux: 2 (0 on master)
Windows: 4 (0 on master)
MacOS : 146 (149 on master)

products component :
Linux: 39 (39 on master)
Windows: 0 (0 on master)

There are new additional compilation warnings on Linux platform:
http://jenkins-test-01.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_17008/job/CR26179-master_build_occt_linux/1/warnings17Result/
Draw_Interpretor.cxx:201, GNU C Compiler 4 (gcc), Priority: Normal
'fd_err_save' may be used uninitialized in this function [-Wmaybe-uninitialized]
Draw_Interpretor.cxx:202, GNU C Compiler 4 (gcc), Priority: Normal
'fd_out_save' may be used uninitialized in this function [-Wmaybe-uninitialized]

There are new additional compilation warnings on Windows platform:
http://jenkins-test-01.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_17008/job/CR26179-master_build_occt_windows_64/1/warnings34Result/
draw_interpretor.cxx:201, MSBuild, Priority: Normal
potentially uninitialized local variable 'fd_err_save' used [d:\builds\vc10\CR26179-master-occt-64\adm\msvc\vc10\TKDraw.vcxproj]
draw_interpretor.cxx:201, MSBuild, Priority: Normal
potentially uninitialized local variable 'fd_err_save' used
draw_interpretor.cxx:202, MSBuild, Priority: Normal
potentially uninitialized local variable 'fd_out_save' used [d:\builds\vc10\CR26179-master-occt-64\adm\msvc\vc10\TKDraw.vcxproj]
draw_interpretor.cxx:202, MSBuild, Priority: Normal
potentially uninitialized local variable 'fd_out_save' used

Regressions/Differences/Improvements:
http://occt-tests/CR26179-master-occt-64/Debian70-64/bugs/fclasses/bug670.html
bugs fclasses bug670: FAILED

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 92132470 / 92249915 [-0.13%]
Total CPU difference: 19034.059999999987 / 19654.729999999847 [-3.16%]
products component :
Total MEMORY difference: 26224469 / 26273564 [-0.19%]
Total CPU difference: 7571.0499999999965 / 7540.919999999997 [+0.40%]

Testing on Windows:
occt component :
Total MEMORY difference: 58095356 / 58098202 [-0.00%]
Total CPU difference: 18347.99521459888 / 18063.496190898888 [+1.57%]
products component :
Total MEMORY difference: 17163441 / 17166598 [-0.02%]
Total CPU difference: 5739.479591299958 / 5624.304052999975 [+2.05%]

There are no differences in images found by testdiff.

mkv

2015-11-11 19:35

tester   ~0048078

Dear rkv,
Branch CR26179 has been rejected due to:
- additional warnings
- regressions/differences/improvements

git

2015-11-13 11:56

administrator   ~0048133

Branch CR26179 has been updated by rkv.

SHA-1: ab2a556a7f95d6bf21b77c73f8204e3554f28bfa


Detailed log of new commits:

Author: rkv
Date: Fri Nov 13 11:56:41 2015 +0300

    0026179: Coding rules - eliminate -Wdeprecated-declarations CLang warnings on tmpnam() usage
    
    Eliminate warnings about uninitialized local variable.

git

2015-11-16 17:45

administrator   ~0048247

Branch CR26179 has been updated forcibly by rkv.

SHA-1: 92808ffe820cd852abae70ffa858fc5089da3541

kgv

2015-11-16 18:14

developer   ~0048249

Dear rkv,

please provide description of last corrections - either in git or at least here in bugtracker.

rkv

2015-11-16 20:24

developer   ~0048254

There is the comment #48133:
"Eliminate warnings about uninitialized local variable."

- appropriate variables are initialized now.

kgv

2015-11-16 20:29

developer   ~0048255

Last edited: 2015-11-16 20:30

Dear rkv,

test report includes
> bugs fclasses bug670: FAILED
and your comment does not specify if this regression has been fixed or not and how.

rkv

2015-11-16 20:36

developer   ~0048256

Last edited: 2015-11-16 21:56

According to apv the test case OCC670 "is not stable" and as it is agreed by Andrey Betenev to run tests again.

kgv

2015-11-16 20:44

developer   ~0048257

Last edited: 2015-11-16 21:56

Please test updated patch.

> According to apv the test case OCC670 "is not stable"
thank you for explanation - it should be here in bugtracker so that others will be able to understand why this regression has been ignored.

git

2015-11-17 16:22

administrator   ~0048297

Branch CR26179 has been updated forcibly by apv.

SHA-1: 684692c1dfcf4bb814a2f100f8ab6c403eaf896e

apv

2015-11-17 16:23

tester   ~0048298

Branch CR26179 has been rebased on the current master

git

2015-11-19 15:20

administrator   ~0048383

Branch CR26179 has been updated by abv.

SHA-1: 1eb65904ebe528298a0da9b3cb32f7d82dc2ac9a


Detailed log of new commits:

Author: abv
Date: Thu Nov 19 15:20:39 2015 +0300

    0000670: Problem with the Print method.
    
    Test for issue 0000670 corrected to avoid unpredictable behavior

apv

2015-11-19 15:32

tester   ~0048386

Dear BugMaster,

Branch CR26179 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 684692c1dfcf4bb814a2f100f8ab6c403eaf896e

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MacOS: 127 (129 on master)
products component:
   Linux: 39 (39 on master)
   Windows: 0 (0 on master)

Regressions/Differences:
Not detected

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 92823781 / 92917230 [-0.10%]
Total CPU difference: 19858.489999999852 / 19784.59999999988 [+0.37%]

Testing on Windows:
Total MEMORY difference: 58081568 / 58096361 [-0.03%]
Total CPU difference: 18516.601095399234 / 18232.414073698874 [+1.56%]

git

2016-04-17 14:14

administrator   ~0053089

Branch CR26179 has been deleted by kgv.

SHA-1: 1eb65904ebe528298a0da9b3cb32f7d82dc2ac9a

Related Changesets

occt: master 95e05159

2015-11-16 14:43:25

rkv


Committer: bugmaster Details Diff
0026179: Coding rules - eliminate -Wdeprecated-declarations CLang warnings on tmpnam() usage

Make a temporary file using BuildTemporary() in "/tmp" folder on Linux or using "TEMP" environment variable on Windows.
Use the new OSD_File::Capture() method for standard output redirection.
Affected Issues
0026179
mod - src/Draw/Draw_Interpretor.cxx Diff File
mod - src/OSD/OSD_Directory.cxx Diff File
mod - src/OSD/OSD_File.cxx Diff File
mod - src/OSD/OSD_File.hxx Diff File

Issue History

Date Modified Username Field Change
2015-04-30 17:39 kgv New Issue
2015-04-30 17:39 kgv Assigned To => kgv
2015-10-14 11:59 kgv Assigned To kgv => rkv
2015-10-14 11:59 kgv Status new => assigned
2015-10-14 11:59 kgv Target Version 7.1.0 => 7.0.0
2015-10-16 10:11 kgv Description Updated
2015-10-16 10:12 kgv Description Updated
2015-10-16 10:12 kgv Description Updated
2015-10-30 14:39 git Note Added: 0047649
2015-10-30 14:41 rkv Note Added: 0047650
2015-10-30 14:41 rkv Assigned To rkv => kgv
2015-10-30 14:41 rkv Status assigned => resolved
2015-10-30 15:11 kgv Note Added: 0047657
2015-10-30 15:11 kgv Assigned To kgv => rkv
2015-10-30 15:11 kgv Status resolved => assigned
2015-11-09 13:42 git Note Added: 0047946
2015-11-09 13:43 rkv Assigned To rkv => kgv
2015-11-09 13:43 rkv Status assigned => resolved
2015-11-09 17:11 kgv Note Added: 0047958
2015-11-09 17:11 kgv Assigned To kgv => rkv
2015-11-09 17:11 kgv Status resolved => assigned
2015-11-09 17:11 kgv Note Edited: 0047958
2015-11-10 09:24 git Note Added: 0047971
2015-11-10 09:24 rkv Assigned To rkv => kgv
2015-11-10 09:24 rkv Status assigned => resolved
2015-11-10 09:53 kgv Note Added: 0047976
2015-11-10 09:53 kgv Assigned To kgv => rkv
2015-11-10 09:53 kgv Status resolved => assigned
2015-11-10 12:00 git Note Added: 0047987
2015-11-10 12:01 rkv Assigned To rkv => kgv
2015-11-10 12:01 rkv Status assigned => resolved
2015-11-10 12:05 kgv Note Added: 0047988
2015-11-10 12:05 kgv Assigned To kgv => bugmaster
2015-11-10 12:05 kgv Status resolved => reviewed
2015-11-10 12:13 mkv Assigned To bugmaster => mkv
2015-11-11 19:34 mkv Note Added: 0048077
2015-11-11 19:35 mkv Note Added: 0048078
2015-11-11 19:35 mkv Assigned To mkv => rkv
2015-11-11 19:35 mkv Status reviewed => assigned
2015-11-11 19:35 mkv Test case number => Not needed
2015-11-13 11:56 git Note Added: 0048133
2015-11-16 17:45 git Note Added: 0048247
2015-11-16 17:45 rkv Assigned To rkv => kgv
2015-11-16 17:45 rkv Status assigned => resolved
2015-11-16 18:14 kgv Note Added: 0048249
2015-11-16 18:14 kgv Assigned To kgv => rkv
2015-11-16 18:14 kgv Status resolved => reviewed
2015-11-16 18:14 kgv Status reviewed => feedback
2015-11-16 20:24 rkv Note Added: 0048254
2015-11-16 20:24 rkv Assigned To rkv => kgv
2015-11-16 20:24 rkv Status feedback => resolved
2015-11-16 20:29 kgv Note Added: 0048255
2015-11-16 20:29 kgv Assigned To kgv => rkv
2015-11-16 20:29 kgv Status resolved => feedback
2015-11-16 20:30 kgv Note Edited: 0048255
2015-11-16 20:36 rkv Note Added: 0048256
2015-11-16 20:36 rkv Assigned To rkv => kgv
2015-11-16 20:36 rkv Status feedback => resolved
2015-11-16 20:37 rkv Note Edited: 0048256
2015-11-16 20:44 kgv Note Added: 0048257
2015-11-16 20:44 kgv Assigned To kgv => bugmaster
2015-11-16 20:44 kgv Status resolved => reviewed
2015-11-16 21:56 abv Note Edited: 0048256
2015-11-16 21:56 abv Note Edited: 0048257
2015-11-17 16:22 git Note Added: 0048297
2015-11-17 16:23 apv Note Added: 0048298
2015-11-18 13:27 bugmaster Assigned To bugmaster => apv
2015-11-19 15:20 git Note Added: 0048383
2015-11-19 15:32 apv Note Added: 0048386
2015-11-19 15:32 apv Assigned To apv => bugmaster
2015-11-19 15:32 apv Status reviewed => tested
2015-11-20 12:07 bugmaster Changeset attached => occt master 95e05159
2015-11-20 12:07 bugmaster Status tested => verified
2015-11-20 12:07 bugmaster Resolution open => fixed
2016-04-17 14:14 git Note Added: 0053089
2016-04-20 15:42 aiv Fixed in Version => 7.0.0
2016-04-20 15:50 aiv Status verified => closed