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

Williams, Norman K norman-k-williams at uiowa.edu
Wed Jan 19 11:54:35 EST 2011


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



More information about the Insight-developers mailing list