drinkmystery / mili

Automatically exported from code.google.com/p/mili
Boost Software License 1.0
0 stars 0 forks source link

Windows warning #22

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Please investigate and address the following warning reported by VisualC++ 2010 
(check that there's no safety issue).

1>c:\dev\mili\include\binary_streams.h(324): warning C4996: 
'std::basic_string<_Elem,_Traits,_Ax>::copy': Function call with parameters 
that may be unsafe - this call relies on the caller to check that the passed 
values are correct. To disable this warning, use -D_SCL_SECURE_NO_WARNINGS. See 
documentation on how to use Visual C++ 'Checked Iterators'
1>          with
1>          [
1>              _Elem=char,
1>              _Traits=std::char_traits<char>,
1>              _Ax=std::allocator<char>
1>          ]
1>          c:\program files (x86)\microsoft visual studio 
10.0\vc\include\xstring(1556) : see declaration of 
'std::basic_string<_Elem,_Traits,_Ax>::copy'
1>          with
1>          [
1>              _Elem=char,
1>              _Traits=std::char_traits<char>,
1>              _Ax=std::allocator<char>
1>          ]
1>          c:\dev\mili\include\binary_streams.h(320) : while compiling class 
template member function 'void 
bistream::_extract_helper<T,IsContainer>::call(bistream *,T &)'
1>          with
1>          [
1>              T=uint32_t,
1>              IsContainer=false
1>          ]
1>          c:\dev\mili\include\binary_streams.h(180) : see reference to class 
template instantiation 'bistream::_extract_helper<T,IsContainer>' being compiled
1>          with
1>          [
1>              T=uint32_t,
1>              IsContainer=false
1>          ]
1>          c:\dev\mili\include\binary_streams.h(188) : see reference to 
function template instantiation 'bistream &bistream::operator >><uint32_t>(T 
&)' being compiled
1>          with
1>          [
1>              T=uint32_t
1>          ]

Original issue reported on code.google.com by danielgutson@gmail.com on 3 Jul 2010 at 5:53

GoogleCodeExporter commented 9 years ago
CNR... I don't have a windows dev environment.
AFAIK the passed values are correct.

Original comment by billybiset on 4 Jul 2010 at 6:09

GoogleCodeExporter commented 9 years ago
Could you please verify?

Original comment by danielgutson@gmail.com on 31 Dec 2011 at 11:25

GoogleCodeExporter commented 9 years ago
this is a warning of unsecure copies that could cause a buffer overrun... 
basically, every copy that doesnt have a maximum amount of items to be copied 
(and its relying in zero termination) will throw this warning. In VS STL, 
exists basic_string::_Copy_s, which is the secure version of the unsecure copy.

Possible solutions:
1) disable the warning for Windows (or better, for VS compiler) and be aware of 
that possibility of a buffer overrun
2) for VS compiler, use string::_Copy_s instead of string::copy, despite it is 
not standard, it will be the secure thing to do
3) set the flag _SCL_SECURE_NO_WARNINGS to disable all these checks and know 
that it will not check for these buffer overruns 

Original comment by esteban....@gmail.com on 5 Jan 2012 at 9:13

GoogleCodeExporter commented 9 years ago
Sure? But the 2nd argument is amount of items, and it's passed:
http://www.cplusplus.com/reference/string/string/copy/

Original comment by danielgutson@gmail.com on 5 Jan 2012 at 10:03

GoogleCodeExporter commented 9 years ago
sorry, I should have said "maximum amount of space in the target"
Basically, VS STL wants to check that the destination buffer is not overrun, 
so, for builk copies and sets, it wants to receive the destination size as 
well...

http://msdn.microsoft.com/en-us/library/x2177ss0(v=VS.100).aspx
This method is potentially unsafe, as it relies on the caller to check that the 
passed values are correct. Consider using basic_string::_Copy_s instead.

Original comment by esteban....@gmail.com on 5 Jan 2012 at 10:18

GoogleCodeExporter commented 9 years ago
Could you please implement option 2, checking that the compiler is VS and using 
the secure version?
(just the #ifdef around those lines)

Original comment by danielgutson@gmail.com on 5 Jan 2012 at 10:22

GoogleCodeExporter commented 9 years ago
Ok, the fancy solution is to do this:

#ifdef WIN32
#ifdef _CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES
    #undef _CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES
#endif
#define _CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES 1
#endif

that will use the secure version and we dont need to change the implementation. 
I want to put this into a place that is general in mili, since any copy (string 
or memory) can raise this error. I dont like to put it in mili.h. What place do 
you suggest to put this?
1) mili.h
2) in binary_streams.h since its there where the warning its raised
3) SConscript (who reported this and what build environment its using?
4) a new config.h file

Original comment by esteban....@gmail.com on 6 Jan 2012 at 12:01

GoogleCodeExporter commented 9 years ago
?
shouldn't you call a different method, accepting an additional argument?

Original comment by daniel.g...@fudepan.org.ar on 6 Jan 2012 at 12:06

GoogleCodeExporter commented 9 years ago
yes, thats the solution, but that macro does it for us. And its the recommended 
method:
http://msdn.microsoft.com/en-us/library/ms175759.aspx

The extra argument is obtained from the target itself, in this case, the 
basic_string::copy target size is obtained from basic_string::size

Original comment by esteban....@gmail.com on 6 Jan 2012 at 1:57

GoogleCodeExporter commented 9 years ago
OK, then pls proceed, either by adding it to binary_streams.h (since mili can 
be used without fbuild by windows users).

Original comment by daniel.g...@fudepan.org.ar on 6 Jan 2012 at 2:34