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.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!