Larry Osterman's WebLog

Confessions of an Old Fogey
Blog - Title

What's wrong with this code?

What's wrong with this code?

  • Comments 29

This one comes courtesy of a co-worker who once upon a time was the lead developer for the Manx (http://www.itee.uq.edu.au/~csmweb/decompilation/hist-c-pc.html) compiler.

He ran into the following problem that was reported by the customer.  They tried and tried to get the following code to compile and it didn't seem to work:

Main.c:

#include <stdio.h>
float ReadFileAndCalculateAverage(FILE *InputFile, int Values[], int LengthOfValues, int *NumberOfValuesRead)
{
   float average;
   int i;

   //
   // Read the Values array from InputFile.
   //

   <Code eliminated for brevity>

   //
   // Now calculate the average.
   //

   for (i = 1 ; i <= *NumberOfValuesRead ; i += 1)
   {
      average += Values[i];
   }

   average = average/*NumberOfValuesRead;
}

int main(int argc, char *argv[])
{
    int Values[20];
    float average;
    int numberOfValues;
    FILE *inputFile;

    inputFile = fopen(“MyFile.Txt”);
    if (inputFile != NULL)
    {
        average = ReadFileAndCalculateAverage(inputFile, Values, sizeof(j) / sizeof(j[0]), &numberOfValues);
    }
    fclose(inputFile);
 }

 Now imagine this problem in a 10,000 line source file :) Have fun trying to track it down.

Edit: Removed bogus <p>'s.  Thank you Newsgator's .Text plugin...

  • /* is a comment, not division followed by pointer dereference :)
  • there is no j.. what is j?

    and /* is not what you think it is there
  • Grr - left the 'j' from a previous version. :)
  • I thought any sane developer uses an editor/IDE with syntax highlighting. Any software capable of this, be it Visual Studio, VIM or even EMACS would show that code as if it was a comment. Pretty trivial if you ask me
  • To my knowledge, vi doesn't, neither do many versions of emacs (some do, some don't).

    Other editors optionally enable syntax coloring.

  • you're probably thinking of VIM, which, when installed, creates a symbolic link vi that actually starts vim with syntax highlighting
  • float average in ReadFileAndCalculateAverage is uninitialized
  • In main, j is undeclared. It seems that j should be Values. Ignoring the eliminated code, it seems that average may be uninitialized. Maybe that was eliminated for brevity. The for loop starts at 1. It should start at 0 and use < rather than <=. After the loop it might be sensible to test for there being 0 values read...

    /* is already pointed out... That is the tricky part, but the other errors are pretty serious.

    Ray
  • the lack of a return statement

    (in addition to the /*)
  • also fopen requires a 2nd parameter specifying the access permitted to "MyFile.Txt"
  • Vim does syntax highlighting. Only a masochist would use vi.
  • Hey, whaddya know, open source works! :)
  • fclose should be inside the if block. I'm not sure what the effect of trying to fclose a NULL pointer is, but I'm not going to take the risk.

  • What about the array? You reserve space for a value[20] array but you don't limit the number of values you read. So when you compute the average you might try to access value[x] where x > 20?
  • Easy. The filename parameter to fopen() uses typographical quotation marks instead of straight quotes.

    That is:
    inputFile = fopen(“MyFile.Txt”);

    should be:

    inputFile = fopen("MyFile.Txt");

    This is what you get when newspaper editors write code ;-)
Page 1 of 2 (29 items) 12