As mentioned in the announcing article, David Maxwell from Coverity ran the bulk of OMGWTF entries through Coverity's Prevent. It would seem that a lot of contestants did not shoot for that defect-to-line ratio or "1", but the static analysis tool did come up with some interesting finds, which may or may not have been intentional.

Here's a summary of the defects in the finalists. The first number is what we considered "actual" defects (such as not freeing memory, etc), where as the second represents all defects (such as warnings about unsafe usage of sprintf, etc):

Entry #100043	Buggy 4 Function Calculator	15 / 77
Entry #100066	FileSystemHashMapNotepadCalculator	45 / 181
Entry #100099	estimator	19 / 21
Entry #100103	TerseCalc	2 / 87
Entry #100123	WTF Web Calc	38 / 86
Entry #100175	OMG!OCRCAL	15 / 78
Entry #100206	Rube Goldberg's Calculator	20 / 132
Entry #100252	Universal Calculator	5 / 62
Entry #100253	Terry's Calculator	5 / 148
Entry #100265	FerronCalc	0 / 35
Entry #100336	VICE, Virtual Integrated Circuit Engine	142 / 222
Entry #100342	ExtensibleCalc	0 / 62

David wrote up some hilights ...

Finalist #11, VICE, Virtual Integrated Circuit Engine topped the list of programs with the highest defect score among the finalists. Capturing the title was a matter of sheer volume in this case. The provided unit tests made room for more defects. Allocating large data structures on the stack, and throwing exceptions that aren't caught anywhere would be bad programming in a real world application, but fortunately this is just for amusement.

 

Here's an example from virtualorgate.cpp:

 

62   	void VirtualOrGate::Set_InputB(const VirtualConnection * input)
63   	{
64   	  if(input == NULL)
65   	  {
66   	    throw std::invalid_argument("input cannot be null");
67   	  }
68   	  InputB = input;
69   	  TransistorB.Set_Base(input);
70   	}

That throw std::invalid_argument() is never caught, right back up through main().

 

 

Finalist #2, FileSystemHashMapNotepad Calculator was king of memory leaks. Each one was small, but is leaked on every calculation, and that means that they, err, 'add up'.

 

381  	float DoAdd(float op1, float op2)
382  	{
383  	
384  		char* op1str = new char[30];
385  		sprintf(op1str, "%.4f", op1 );
386  	    char* op2str = new char[30];

There's no cleanup at the end of the operation functions, and no deletion of those memory allocations.

 

In third place for defect score is Finalist #5, WTF Web Calc, here's a sample from wtfcalc.c:

 

75   	        clientsock = accept(bindsock,(struct sockaddr*)&incoming, &ilen);
76   	        printf("accepted %d\n", clientsock);
77   	        
At conditional (1): "done == 0" taking true path
78   	        while(!done) 
79   	        {   char c[2] = {0,0};;
Event negative_returns: Tracked variable "clientsock" was passed to a negative sink.
80   	            if(read(clientsock,&c,sizeof(c)-1) != 1) break;
81   	            len++;
82   	           
83   	            strcat(line,c);
84   	            if (c[0] == '\n') {
85   	                printf("rcvd %d: %s",nline,line);
86   	                if(!nline++) strcpy(get,line);
87   	                
88   	                memset(line,0,sizeof(line));
89   	                if(len == 2) {
90   	                    printf(">> end of req\n");
91   	                    done = 1;
92   	                }
93   	                len=0;
94   	                
95   	            }
96   	            
97   	        }

If the call to accept fails, it will return an error code, as a negative number. The code doesn't handle that case, and passing a negative value as the first argument to read will always fail (Invalid descriptor). When the read fails, the program will break out of the while loop, but then it will carry on processing the 'input' which was never read...

 

Finalist #7, Rube Goldberg Calculator has some memory leaks as well.

 

From calcfunc.cpp:

 

43   	float DoSub(float op1, float op2)
44   	{
45   		XmlServer xml = XmlServer();
46   		xml.Open(defaultXmlPath);
47   		Fraction fraction1 = Fraction(op1);
48   		Fraction fraction2 = Fraction(op2);
49   		Fraction* result = xml.ReadFraction(fraction1, fraction2, XmlServer::FunctionType::Subtract);
50   		if(result == NULL)
51   		{
52   			xml.WriteFraction(fraction1, fraction2, XmlServer::FunctionType::Subtract, fraction1 - fraction2);
53   			xml.Close();
54   			return DoSub(op1, op2);
55   		}
56   		return result->ToFloat();
57   	}

 ReadFraction allocates storage for the Fraction, and DoSub owns that pointer once it is returned, but returning result->ToFloat() returns the value, not the address, and after that it's too late to clean up. Points should be awarded for consistency though, since the same leak exists in all the other operations implemented too. Rube would be proud.

Alex Papadimoulis:

 

Finalist #7, Rube Goldberg Calculator has some memory leaks as well.

 

From calcfunc.cpp:

 

43   	float DoSub(float op1, float op2)
44   	{
45   		XmlServer xml = XmlServer();
46   		xml.Open(defaultXmlPath);
47   		Fraction fraction1 = Fraction(op1);
48   		Fraction fraction2 = Fraction(op2);
49   		Fraction* result = xml.ReadFraction(fraction1, fraction2, XmlServer::FunctionType::Subtract);
50   		if(result == NULL)
51   		{
52   			xml.WriteFraction(fraction1, fraction2, XmlServer::FunctionType::Subtract, fraction1 - fraction2);
53   			xml.Close();
54   			return DoSub(op1, op2);
55   		}
56   		return result->ToFloat();
57   	}

 ReadFraction allocates storage for the Fraction, and DoSub owns that pointer once it is returned, but returning result->ToFloat() returns the value, not the address, and after that it's too late to clean up. Points should be awarded for consistency though, since the same leak exists in all the other operations implemented too. Rube would be proud.

 I had no idea my program leaked memory :) Bonus WTF.  Since I've been doing nothing but C# for over a year and a half now, I forgot the finer points of memory management.  See, in C# you can compare objects themselves to null.  I was having problems figuring out how to do that in C++, so I just made it return a pointer instead of an object, and figured the object would dispose itself once the pointer went out of scope. The compiler didn't complain so I didn't look into it. Oops!

Alex Papadimoulis:

Finalist #11, VICE, Virtual Integrated Circuit Engine topped the list of programs with the highest defect score among the finalists. Capturing the title was a matter of sheer volume in this case. The provided unit tests made room for more defects. Allocating large data structures on the stack, and throwing exceptions that aren't caught anywhere would be bad programming in a real world application, but fortunately this is just for amusement.

 

Here's an example from virtualorgate.cpp:

 

62   	void VirtualOrGate::Set_InputB(const VirtualConnection * input)
63   	{
64   	  if(input == NULL)
65   	  {
66   	    throw std::invalid_argument("input cannot be null");
67   	  }
68   	  InputB = input;
69   	  TransistorB.Set_Base(input);
70   	}

That throw std::invalid_argument() is never caught, right back up through main().

 In my defense, the uncaught exceptions were entirely sanity checks like this one that didn't *need* to be caught, since I knew there was no way the input could be NULL.

 I did, however, deliberately choose stack-based allocation, and I stand by it. There's nothing wrong with a large stack allocation it at all, if you know what you're doing.