Hi Brian,
> There's a Windows function called vsnprintf_s() that
> might solve the null termination problem ...
> was wondering if you had heard of it and if
> you had tried it.
Yes, and no, until now... although I did consider it
earlier, and rejected it as MORE work!
There are so many, _MANY_ cases where MS, in its infinite
wisdom, has provided NEW, so called 'secure', functions,
usually with _s appended... but always with an additional
'count' parameter...
That is what the config.h :-
#pragma warning(disable:4996)
is all about ;=)) The MS compiler outputs a 'depreciation'
warning, on lots and LOTS of functions like strlen -> strnlen_s,
printf -> printf_s, etc, etc, etc... where you must also
specify a MAXIMUM length to count/use, as an additional
1 (or more) parameter(s)...
I can see 'some' benefit in the case of 'strlen', to stop
it counting after the buffer length, but since
vsnprintf() already has a controlling 'n' maximum
buffer length, I can not see how they can make it _MORE_
secure ;=((
In most cross-platform open sources, these _s (or n) MS
ONLY functions are usually ignored, or else the platform
code would be _FILLED_ with :-
#if (defined(_MSC_VER) && (_MSC_VER >= 1300)) // *
len = strnlen_s(buf, some_count);
len = sprintf_s(buf, count, format, ...);
#else
len = strlen(buf);
len = sprintf(buf, format, ...);
#endif
Note it can NOT be implemented as a macro, since the
'count' requires some knowledge of the specific code
at that place...
* Not sure if this is the correct compiler level
where the 'depreciation' and _s (& n) functions arrived,
but it was around here... early 2000 anyway...
On reading the MS MSDN vsnprintf_s it seems to read
nearly the same as the vsnprintf function, except it
does return -1 if the formatted string is exactly
equal to the supplied count, provided _TRUNCATE
is used for the second count, at least warning there is
no room for the null character!
We have to use _TRUNCATE as the 2nd count, or else an
'invalid parameter handler' would pop up, with 'Abort,
Continue, ...etc if the formatted data plus terminating null
exceeded or equaled the count, which is _NOT_ what we want...
So, in this case, since we ALREADY have to have a _MSC_VER
switch anyway, to account for the -1 return, then below is
the new function... in this case, the vsnprintf_s
re-write has slightly 'improved' the function...
But I also can not see why we did not go with what I had
already VERY CAREFULLY researched, decided and tested...
there seems nothing wrong with always doing :-
_buf[_size - 1] = 0;
in the windows only code...
And on a similar note...
> ... I actually never apply patches directly using
> the 'patch' command. Instead, I go through the patch
> file, look at the suggested changes, decide if they're
> necessary or not, modify them as I see fit, ...
Yes, this is what I suspected ;=))
Not to complain really, but it is not much fun to
'outsiders' like myself to always have a carefully
considered patch, re-patched like this ;=()
I usually take considerable care to duplicate the style
of the particular code, rather than using my own standards.
I would be nice if once in a while I could 'see' MY
patch in the code... and not your patch of my patch.
And this becomes especially true if you are going to
second guess me in the windows environment... The
vsnprinf patch I finally submitted should have been
acceptable, rather than you spending time on MSDN
documentation as well...
As stated, we in the window IDE are perpetually reminded
of these 'alternate' functions every time we consult the
documentation, F1, or forget to suppress 4996... this is one
of the first times I have noted a subtle 'improvement'
in functionality...
And imagine this in reverse, if every time you wanted to
make a cvs change, another maintainer, even subtly, changed
your code to what they thought was slightly 'better',
in code content, style or even comments...
Anyway, I agree YOU are the 'maintainer', and I am
NOT, although I would like to be... so you have
every right to re-write what I suggest ;=)) Just
recognize that this dampens my 'enthusiasm'... and
would imagine this being true in some other cases,
but not all...
Regards,
Geoff.
<patch file="misc.cxx">
#ifdef _MSC_VER
// windows vsnprintf() returns -1, if it can NOT fit the formatted
// string within the 'count', and most certainly _NOT_ the needed length!
// And no need to 'copy' the 'va_list ap' pointer, or 'dispose' of it.
// The ONLY failure here would be if 'realloc' FAILED, so 'exit(1)'
int newLen;
while ((newLen = vsnprintf_s(_buf + _strlen, _size - _strlen,
_TRUNCATE, fmt, ap)) < 0) {
_size += _increment;
_buf = (char *)realloc(_buf, _size);
if ( !_buf ) {
fprintf(stderr,
"ERROR: MEMORY REALLOCATION FAILED ON %d BYTES! Aborting...\n",
_size);
exit(1);
}
}
_strlen += newLen; // bump the length, by this 'format' length...
#else
</patch>
|