Looking at the code
Lets make it readable by indenting more than one character! Two is acceptable but more usall is around four.
OK we assume this is reading from some file descriptor but it is not a standard function.
int length = fileLength(fpin);
The worst problem with C code is that people don't check the error codes from system function calls. ALWAYS check the error code. There may not be mcuh you can do (apart from log a message). BUT it is nice to know that you crashed because you ran out of memory rather than there being a bug in your code. Since you are using functions that assume the buffer is correctly terminated I would add an extra character to the buffer and stick '\0' into the location.
char *buffer = (char *) malloc((length+1) * sizeof(char));
// Add
if (buffer == NULL)
{ printf(stdout, "malloc() failed\n";
exit(1);
}
buffer[length] = '\0'
The fread() function may fail you need to check the amount read is what you expected (also you should probably re-name to make it easier to understand).
int readObjects = fread(buffer, sizeof(char), length, fpin);
// Add
if (readObjects != length)
{
if (feof(fpin)) { /* Report unexpected EOF */ }
if (ferror(fpin)) { /* Report ERROR */}
}
The realloc() function is very often used incorrectly. It it returns NULL when it failed to re-alloc a bigger space (but then the old memory is not freed). So you must catch the result of realloc into a temporary and only swap with your buffer if non NULL. Since we don't know if the realloc will work don't update your allocated value until you know it has worked.
if (allocated == *elementCount) {
allocated *= 2;
// Reallocate space based on need
realloc(userInput, allocated * sizeof(int));
}
// Should be written like this:
if (allocated == *elementCount)
{
int* tmp = realloc(userInput, allocated * sizeof(int) * 2);
if (tmp == NULL)
{
printf("realloc() failed\n";
exit(1);
}
allocated *= 2;
userInput = tmp;
}
According to the documentaion atoi() has been deprecated in favor of strtol()
, and is equivalent to (int)strtol(str, (char **)NULL, 10)
So you should probably use that. And another point to check for errors.
userInput[(*elementCount)++] = atoi(tokenized);
Technique used.
I would not have bothered to read the data into its own buffer (then tokenize it). The C library has a perfectly good stream reading library in scanf(). The same functionality can be achieved using:
int value;
while(fscanf(fpin, " %d",&value) == 1)
{
// We read a value now store it.
}
Note the format string: " %d"
The leading space ignores multiple (including zero) white space characters proceeding your data (Note: White space includes '\n' so it will correctly wrap past the end of line). Then the %d will read a number from the stream. If it succeeds then it will return 1 to indicate the number of decoded '%' objects is 1.