In general, it is better to post the actual problematic code
(preferably after shrinking it down to a "problematic nub", as it
were), but a URL reference can work if the one reading netnews
bothers to follow the link.
I have 2 malloc's in my program, and when I write the contents of them to
the screen or to a file, there aren addition 4 characters.
As far as I can tell, both the code to register the malloc and to write
information into the malloc is solid. Why then ismy program returning an
additional 4 characters?
It is not quite as solid as one might hope, although the problem
has nothing to do with malloc() per se. Here are excerpts from the
code (quoted with ">" as usual, although I had to insert the markers
myself):
// soCrypt 1.0
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
// #include <md5.h>
OK so far, although //-comments are specific to C99. You have
included necessary headers, so you will not need to cast malloc()'s
return value.
// Global variables
int statCode = 0; // Status code
int mode; // Mode variable (1 = enc, 2 = dec)
int i; // Looper variable
int inSize = 0; // Input filesize
int intRand; // Random int
char tmp_char; // Temporary char
Many of these should not be file-scope external-linkage ("global")
variables, although this is mostly a style issue (at least in a
program this small).
Note that tmp_char has type "char"; on a typical PowerPC, it would
hold values between 0 and 255 inclusive, because there plain "char"
is unsigned. The variable inSize is a plain (signed) int and has
at least the range [-32767..+32767] (although most systems, today,
have an even wider range, about +/- 2 billion).
char *pMasterKey; // Malloc pointer to the master key
char *pInputData; // Malloc pointer to the input data
Skipping forward, we have:
// Reads the input file
int readFile()
{
rewind(inFile);
i = 0;
tmp_char = 'a';
while(tmp_char != EOF)
{
i++;
tmp_char = getc(inFile);
}
inSize = i-1;
This loop tries to count the size of the file by calling getc()
until getc() returns EOF. The problem is that EOF is some sort of
negative number -- typically -1, but perhaps even -2000 or some
such -- and tmp_char is a plain "char". If tmp_char is unable to
hold the value EOF, which will be true if plain char is unsigned
or if EOF is less than CHAR_MIN (e.g., -2000 vs -128 for instance),
the loop will never terminate.
This is why getc() returns a value of type "int" in the first place,
so that it can return all possible "char"s (having first converted
any negative ones to positive values as if via "unsigned char"),
yet also return the special marker value EOF. If you want to store
both "any valid character" *and* EOF, you need something with a
wider range than "any valid character".
Of course, there is really no need for tmp_char at all, nor for
correcting for the off-by-one error produced by counting inside
the loop *before* getting a character. Just change the loop to,
e.g.:
while (getc(inFile) != EOF)
i++;
Combine this with using local variables, and perhaps a "for"
loop to collect up the initialization, test, and increment, we
might get something like:
int readFile() {
int i;
rewind(inFile);
for (i = 0; getc(inFile) != EOF; i++)
continue;
inSize = i;
(although I would also pass the "FILE *" parameter to readFile,
and probably return the allocated memory rather than an "int"
status code).
if ((pInputData = (char *)malloc(inSize * sizeof(char))) == NULL)
{
statCode = 8;
return(statCode);
}
This is OK in and of itself, but there are two important things to
note. First, the cast is not required. It does no harm, but also
does no help. It is a bit like saying "tmp_char = (char)getc(inFile)",
when tmp_char is already a char. The assignment will do the
conversion for you -- and you do not use a cast below, so why use
one above?
Second, and the actual source of the observed problem later, note
that this allocates just enough space to store all the characters
you intend to read from the file.
Consider the C string "hello world". How many characters are in
it? How many characters does it take to *store* it? Why, after:
char hello[] = "hello world";
is there a difference of 1 between "sizeof hello" and "strlen(hello")?
The answer is: because C strings require a '\0' marker after all
their valid "char"s. The array hello[] has size 12, not size 11,
because it stores the 11 "char"s that make up the two words and
the blank, and then one more to store the '\0' marker.
The variable inSize might (for instance) hold 5 if the file contents
are "word\n" (perhaps followed by an EOF marker, if your system
actually uses such markers in files), but if you want to use the
sequence {'w', 'o', 'r', 'd', '\n'} as a C string, you need *six*
bytes: {'w', 'o', 'r', 'd', '\n', '\0'}.
Of course, there is no requirement that you treat the file as
a C string -- that part is up to you. In any case:
rewind(inFile);
i = 0;
tmp_char = 'a';
while (i < inSize)
{
tmp_char = getc(inFile);
*(pInputData + i) = tmp_char;
i++;
}
return 0;
}
There is nothing *wrong* here, but the code can be simplified
enormously. First, tmp_char is never inspected without first
calling getc(), so there is no need to "prime the pump" -- the
loop tests "i < inSize". Second, there is no need for tmp_char
at all; you can just assign the value from getc() directly into
pInputData
. Third, you can write pInputData that way,
rather than using the equivalent unary-"*" sequence, and again
perhaps a "for" loop might express the whole sequence better:
rewind(inFile);
for (i = 0; i < inSize; i++)
pInputData = getc(inFile);
return 0;
}
Skipping forward to the source of the observed problem:
// Writes the output file
int writeFile()
{
fputs(pInputData, outFile);
fputs(pMasterKey, keyOut);
return 0;
}
The fputs() function demands a string -- a sequence of "char"s
ending with a '\0' termination marker. pInputData points to the
first of a sequence of "char"s, but not one that has the "stop here
at the \0" mark in it.
Without making any other changes, you can either allocate one extra
byte and put in the '\0', or you can change the method you use to
write the final output. The two simply have to agree as to whether
pInputData (and pMasterKey -- but I did not even look at that code)
is a counted string (length inSize, for pInputData) or a C-style
'\0'-terminated string.
Note that a '\0'-terminated string cannot *contain* a '\0', so if
you want (for whatever reason) to allow embedded '\0' bytes, you
will have to choose the counted-string method. You could write
out a counted string with a loop:
int writeFile() {
int i;
for (i = 0; i < inSize; i++)
putc(pInputData, outFile);
/* optional:
if (fflush(outFile) || ferror(outFile))
... handle output failure ... */
/* and repeat for pMasterKey */
}
or you can use fwrite(), which essentially does the loop for you.
(Note that fwrite() works just fine with ordinary text, and in
fact, fputs() can be implemented internally as:
int fputs(char *s, FILE *stream) {
size_t len = strlen(s);
return fwrite(s, 1, len, stream) == len ? 0 : EOF;
}
Here strlen() looks for, but does not count, the terminating '\0',
then fwrite() loops over all the valid bytes, putc()ing each one
to the stream. The only remaining problem is that the return value
from fwrite() does not match that from fputs(), so it has to be
converted.)
