[Insight-developers] snprintf portability (was: Fix possibility of buffer overflow in itkNumericSeriesF...)

[Bill Lorensen]

I think the current gerrit patch will work fine on the compilers that
ITK supports.
http://review.source.kitware.com/#change,521

Bill

On Wed, Jan 19, 2011 at 11:54 AM, Williams, Norman K
<norman-k-williams at uiowa.edu> wrote:
> Given all the trouble around snprintf -- and I would NEVER have gone down
> that road if I'd known what a mess it was going to make -- I wonder if we
> should consider CHANGING THE FRICKING CLASS.
>
> It wasn't a great idea in the first place to use a printf format string as
> an argument to an ITK class method.  C++ is theoretically supposed to be a
> post-printf world*  It exposes the wrong level of abstraction. It's an
> invitation to users to diddle the underlying implementation, not an
> explicit contract of promised behavior.
>
> As a 'coffee' cup proposal, maybe something like this would be at a more
> appropriate level of abstraction
>
> /** the part of the filenames preceding the file number */
> void SetPrefix(const std::string &prefix);
> /** the part of the filenams following the file number */
> void SetSuffix(const std::string &suffix);
> /** how many digits, minimum to represent the file number */
> void SetIntegerWidth(int width);
> /** whether to zero-pad the file number */
> void ZeroPad(bool pad);
>
> *of course, people still use printf formatting in C++, and I don't even
> know if standard C++ even provides all the formatting options available by
> printf format strings. It's a significant failure of the C++ language &
> library that the standard hasn't comprehensively made printf obsolete, in
> a completely portable, well defined way.
>
> On 1/19/11 10:01 AM, "Brad King" <brad.king at kitware.com> wrote:
>
>>On 01/19/2011 10:49 AM, Code Review wrote:
>>> From Bill Lorensen <bill.lorensen at gmail.com>:
>>> I think a better job could be done for windows. Currently, the first
>>>snprintf
>>> will always return -1 for windows.  If char c[10000]; was used instead
>>>of char c;
>>> most of the time (except for huge filenames) snprintf will return a
>>>valid value
>>> on all platforms.
>>> To view, visit http://review.source.kitware.com/521
>>
>>In that case we might as well just use that result directly and only
>>fall back to dynamic allocation when the stack buffer isn't big enough.
>>
>>For reference, I just experimented with snprintf and _snprintf on a
>>bunch of compilers:
>>
>>  - HP
>>  - SunPro
>>  - SGI MipsPro
>>  - IBM XL
>>  - GNU on Linux
>>  - Intel on Linux
>>  - Intel on Windows
>>  - VS 6, 7, 7.1, 8, 9, 10
>>  - Borland 5.5, 5.6, 5.8
>>  - Watcom
>>  - GNU on Windows (MinGW)
>>  - GNU on Cygwin (default, -mwin32, -c99, -mwin32 -c99)
>>
>>The attached snprintf.c shows the results I encountered.  The most
>>interesting was on SGI where -D_XOPEN_SOURCE=500 changes the result.
>>Only cases in which the buffer is large enough are consistent everywhere.
>>
>>-Brad
>
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.html
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
>