Archived issue #0026179
Coding rules - eliminate -Wdeprecated-declarations CLang warnings on tmpnam() usage
Description
The following warnings should be eliminated:
gcc 4.9.2 on Linux generates similar warning:
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 reproduce
N/A
Public activity
23 archived notes
Participants are labeled by their role within this record.
Branch [archived branch] has been created by Participant.
[revision removed]
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
[revision removed]
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
mkstemp() and mkdtemp() are used instead of tempnam().
+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.
Branch [archived branch] has been updated forcibly by Participant.
[revision removed]
[revision removed]
-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.
Branch [archived branch] has been updated forcibly by Participant.
[revision removed]
[revision removed]
> Branch [archived branch] has been updated forcibly by Participant.
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 should not be used within inline methods - please drop the keyword.
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")
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")
Branch [archived branch] has been updated by Participant.
[revision removed]
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.
[revision removed]
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.
Please test the patch.
Dear Commenter 1,
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.
[revision removed]
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.
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.
[revision removed]
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.
Dear rkv,
Branch [archived branch] has been rejected due to:
- additional warnings
- regressions/differences/improvements
Branch [archived branch] has been rejected due to:
- additional warnings
- regressions/differences/improvements
Branch [archived branch] has been updated by Participant.
[revision removed]
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.
[revision removed]
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.
Branch [archived branch] has been updated forcibly by Participant.
[revision removed]
[revision removed]
Dear rkv,
please provide description of last corrections - either in git or at least here in bugtracker.
please provide description of last corrections - either in git or at least here in bugtracker.
There is the comment #48133:
"Eliminate warnings about uninitialized local variable."
- appropriate variables are initialized now.
"Eliminate warnings about uninitialized local variable."
- appropriate variables are initialized now.
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.
test report includes
> bugs fclasses bug670: FAILED
and your comment does not specify if this regression has been fixed or not and how.
According to apv the test case OCC670 "is not stable" and as it is agreed by Andrey Betenev to run tests again.
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.
> 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.
Branch [archived branch] has been updated forcibly by Participant.
[revision removed]
[revision removed]
Branch [archived branch] has been rebased on the current master
Branch [archived branch] has been updated by Participant.
[revision removed]
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
[revision removed]
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
Dear Commenter 1,
Branch CR26179 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]
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%]
Branch CR26179 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]
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%]
Branch [archived branch] has been deleted by Author.
[revision removed]
[revision removed]