[ad_1]
Gavin inherited some "very old" C code, originally developed for old Windows systems. The code is loaded with a number of reinvented wheels.
For example, they needed to do a case insensitive string comparison. Now, instead of using stricmp, the case insensitive string comparison, they wrote this:
int uppercmp( char *sStr1, char *sStr2 )
{
char *sUStr1;
char *sUStr2;
int iRet;
if ( ( sStr1 == NULL ) || ( sStr2 == NULL ) ) return -1;
__try
{
sUStr1 = strupr(strdup( sStr1 ));
sUStr2 = strupr(strdup( sStr2 ));
iRet = strcmp( sUStr1, sUStr2 );
return iRet;
}
__finally
{
FreeIfNotNULL(sUStr1);
FreeIfNotNULL(sUStr2);
}
return
}
This uses Microsoft's structured exception handling extensions to C, hence the __try. SEH is its own weird beast, with huge amounts of caveats and gotchas and weirdness. It's honestly more interesting than this function, which isn't itself a WTF- it's just bad. They duplicate the input strings, convert them to upper case, and then compare. The real ugly thing is the return value- a -1 if either input string is NULL, but also a -1 from strcmp if sUStr1 < sUstr2. And of course, an empty return if anything causes an exception.
This code was so nice, they wrote it twice, as here's their StringEqual function.
BOOL StringEqual( IN char *sSearch,
IN DWORD dwSearchLen,
IN char *sToken,
IN DWORD dwTokenLen,
IN BOOL bCaseInsensitive )
{
char *sUpperSearch;
char *sUpperToken;
BOOL bStringEqual = FALSE;
// If the lengths are mismatched or the parameters NULL, its not a match
if ( (dwSearchLen < dwTokenLen) || (sToken == NULL) || (sSearch == NULL) )
{
return FALSE;
}
// Check for case insensitivity
if ( bCaseInsensitive )
{
__try
{
// Make upper case versions of the comparitor and the comparitand (my own words ;)
sUpperSearch = strdup( sSearch );
sUpperToken = strdup( sToken );
_strupr( sUpperSearch );
_strupr( sUpperToken );
// And return a TRUE if there is a match
return ( strncmp( sUpperSearch, sUpperToken, dwTokenLen ) == 0 );
}
__finally
{
FreeIfNotNULL(sUpperSearch);
FreeIfNotNULL(sUpperToken);
}
}
else
{
// Return if there is a case sensitive match
return ( strncmp( sSearch, sToken, dwTokenLen ) == 0 );
}
// This line never gets called, but lets keep it here to make the
// warning go away
return FALSE;
}
Here, they're wrapping strncmp, which expects the caller to supply the length of the strings (instead of relying on null terminators). That's arguably a good practice, but it's foiled by using strdup, and _strupr– which are looking for null terminators.
With all these homebrew string handling functions, would you be shocked to know they have a homebrew memory management function too? Here's OverlappedMemcpy.
void OverlappedMemcpy( IN OUT void *vDest,
IN void *vSrc,
IN DWORD dwLen )
{
void *vTemp;
__try
{
// Store the from store
vTemp = malloc( dwLen );
// Copy the source
memcpy( vTemp, vSrc, dwLen );
// .. to the dest through the temporary intermidiate
memcpy( vDest, vTemp, dwLen );
}
__finally
{
FreeIfNotNULL(vTemp);
}
}
I do not know what about this is "overlapped". It's just a regular memcpy where we added an intermediate step of copying into a temporary buffer before copying to the destination buffer. So it's just really inefficient, for absolutely no benefit to anyone.
.comment { border: none; }
Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!
[ad_2]
Remy Porter
Source link
