problem with malloc()

Hi,

I’m writing a program that will encode and decode uue files and I’ve finished the encoder and I am now on to the decoder. I normal encoded uue file will look something like this…

begin 644 foo.txt
$8V%T<P``
`
end

where “foo.txt” should be the name of the file after decoding. Currently I am at the point where I am trying to take out the “foo.txt” by creating a char * and using malloc on whatever size the filename is. Unfortunately when I run my debugger it appears that malloc isn’t correctly allocating the amount of memory I want. Here’s the code I’ve got right now.

Code:

---

#include <stdio.h>
#include <malloc.h>
#include "uucode.h"

#define MAX_BUFFER 45
#define ERROR -1
#define CARRIAGE_RETURN 13
#define START_OF_FILENAME 10

/* encoding function taken out to conserve space */

int uudecode(const char *InputFilename)
{
        FILE* eFile = fopen(InputFilename, "r");
        FILE* oFile = NULL;
        char buffer[MAX_BUFFER];
        char *outFile;
        int i, chRead, endOfFirstLine = 0;

        /* if there was an error openning the encoded file */
        if(!eFile)
                return ERROR;

        chRead = fread(buffer, 1, MAX_BUFFER, eFile);

        /* finding the end of the first line of encoded text to find the filename */
        for(i = 0; i < MAX_BUFFER && *(buffer + i) != '\n' && *(buffer + i) != CARRIAGE_RETURN; i++)
                endOfFirstLine++;

        /* setting the filename size */
        outFile = (char *)malloc(endOfFirstLine - START_OF_FILENAME);

        /* if malloc didn't work return an error */
        if(outFile == NULL)
                return ERROR;

        /* copying the filename from the buffer to outFile */
        for(i = START_OF_FILENAME; i < endOfFirstLine; i++)
        {
                *(outFile + i) = *(buffer + i);
        }

        oFile = fopen(outFile, "w");

        if(!oFile)
                return ERROR;

        fclose(eFile);
        fclose(oFile);

        /* free is returning a memory error
        free(outFile); */
       
        return 0;
}

---

If I put the encoded file I showed above through this code I get endOfFirstLine = 17 while START_OF_FILENAME is #defined to be 10 which means a malloc(7) which should be correct I’m assuming, although I’m starting to doubt it since when I run free(outFile) I get a “DAMAGE: after normal block (#52) at …..” error.

Also looking at my debugger the contents of outFile are "ÍÍÍÍÍÍÍýýýý««««««««þîþîþ" before putting anything in to it which is way too big.

If I comment out the free(outFile) line it is able to create a file by the name of “ÍÍÍÍÍÍÍýýýfoo.txt««þîþîþ” so I know I’m able to copy the filename in there.

My problem is that I don’t understand what is going on with malloc() or why “foo.txt” appears in the middle of outFile. Or why free is returning an error for that matter.

Any help that someone could bring to me would be greatly appreciated, thanks for taking the time to read this.

-Peter

PS I'm using Visual Studio 2003 .net

The implementations of malloc and free are correct. Somewhere else in your program, you're writing to a place in memory that you have not correctly allocated. If you write outside the bounds of any dynamically allocated array, or dereference an unitialized pointer, you're liable to write over part of the datastructure which manages the blocks of memory on the heap.

So find out where you're doing that.

Why not just use fgets to read the first line?

"foo.txt" requires 8 characters.

Strings must be null terminated.

Don't cast the value returned by malloc in C.

[edit]malloc is declared in <stdlib.h>.

Quote:

---

Originally Posted by Rash

The implementations of malloc and free are correct. Somewhere else in your program, you're writing to a place in memory that you have not correctly allocated. If you write outside the bounds of any dynamically allocated array, or dereference an unitialized pointer, you're liable to write over part of the datastructure which manages the blocks of memory on the heap.

So find out where you're doing that.

---

Thank you... that was a stupid mistake. free() works fine now but I am still creating a file called "foo.txtýýýý««««««««þîþîþ".

Am I allocating too much memory?

Just for referece I changed one of my for loops to

Code:

---

        /* copying the filename from the buffer to outFile */
        for(i = START_OF_FILENAME, j = 0; i < endOfFirstLine; i++, j++)
        {
                *(outFile + j) = *(buffer + i);
        }

---

Thanks again.

>>"foo.txtýýýý««««««««þîþîþ".<<

Quote:

---

Originally Posted by Dave_Sinkula

Strings must be null terminated.

---

Quote:

---

Originally Posted by Dave_Sinkula

Why not just use fgets to read the first line?

"foo.txt" requires 8 characters.

Strings must be null terminated.

Don't cast the value returned by malloc in C.

[edit]malloc is declared in <stdlib.h>.

---

Thanks, my string is now null terminated and everything is gravy.

Thanks again everyone, stupid problem. I'm kicking myself because I know I should have known both of these things...

PS I'm not familiar with fgets but I'll look in to it, thanks.

Looking in to it I'm avoiding using fgets because I don't see a way to pick where I start getting characters out of my FILE *.

>Looking in to it I'm avoiding using fgets because I don't see a way to pick where I start
>getting characters out of my FILE *.

Carefully study the fseek() function for in the future.

Quote:

---

Originally Posted by citizen

>Looking in to it I'm avoiding using fgets because I don't see a way to pick where I start
>getting characters out of my FILE *.

Carefully study the fseek() function for in the future.

---

thanks!

I've taken out my hideous for loop and I'm now using fseek() and fgets()

thanks again!

Quote:

---

the contents of outFile ... before putting anything in to it which is way too big.

---

As a side note: malloc() is not required to return a block of memory the size you requested. malloc() will return a block of memory (on success) of at least the size you requested. This means that your block could be bigger than what you asked for. (But you shouldn't use any more than what you asked for as you're not guarenteed that it will be there.)

Of course, on failure, malloc() returns NULL.

One final comment. In your code, you provide error handling for various things, out of memory, file failure, etc. These appear to merely exit the function, which in turn leaks resources. You error handlers need to clean up: close files that the function needs to close, deallocate memory, ect. Ask yourself: if I allocate this resource, and my function fails at some point, at the point where it fails, is this allocated resource taken care? (Either purposefully left to the caller to free or free'd by the function?)

Quote:

---

Originally Posted by Cactus_Hugger

As a side note: malloc() is not required to return a block of memory the size you requested. malloc() will return a block of memory (on success) of at least the size you requested. This means that your block could be bigger than what you asked for. (But you shouldn't use any more than what you asked for as you're not guarenteed that it will be there.)

Of course, on failure, malloc() returns NULL.

One final comment. In your code, you provide error handling for various things, out of memory, file failure, etc. These appear to merely exit the function, which in turn leaks resources. You error handlers need to clean up: close files that the function needs to close, deallocate memory, ect. Ask yourself: if I allocate this resource, and my function fails at some point, at the point where it fails, is this allocated resource taken care? (Either purposefully left to the caller to free or free'd by the function?)

---

Thanks for the info, I've learned a bunch from this whole post. I'll take a look at my error handling now that I think I've got all the decoding working.

Thanks again everyone!