the basics of malloc

I've enjoyed many of your posts and decided to register and pick your minds if you don't mind.

Somehow I've managed to program for a LONG time and never bothered to understand some things. All too often I've done "just make it work" programming.

I'm working on a program now and noticing things I just think I should understand. For example, here's a line from a program:

note[ctr].nBody = (char *) malloc (100 * sizeof( char ) ) ;

The structure for note is:

Code:

---

typedef char *STRING

typedef struct noteinfo {              /* structure of note                */
        int    nSize;                                /* number of characters in note body        */
        STRING nBody;                          /* text of note                                        */
        STRING nExtra;                          /* area for extra info such as date string        */
} NOTEINFO;

---

So, later when I'm ready to write the file I wanted to test the note body (note[ctr].nBody) to see if it was empty. I expected to be able to see if it was empty by using

Code:

---

if( note[ctr].nBody == NULL )

---

But it turns out that there are four chars in .nBody even though I've done nothing with it since the malloc and sizeof() also returns a size of 4.

Can anyone tell me what malloc puts in the string during malloc? And, how might I test note[ctr].nBody to see if it is empty? (Empty meaning nothing has been done to it since the malloc.) I've tested for content using lstrlen(), which returns 0. But that seems kinda sloppy.

Thanks,
Dale L

Using sizeof on a pointer will typically return 4 or 8, depending on the size of a pointer on the system. So in other words, you are getting the size of the pointer, and not the actual value or data stored there.
But if you do sizeof(*nBody), you will get 1 ( sizeof(char) ), because the compiler cannot know how many elements or how big the data block pointed to by the pointer is.
Malloc only returns NULL if it fails, so you can't check for NULL either.
Therefore, the better solution is to keep track of whether or not something has been written. But you do have a "nSize" variable, so use that (ie nSize == 0).

Also beware that typedefing a pointer may not be the greatest idea. It will probably introduce confusion, so avoid that.
And "n" isn't a typical prefix for a string. It's more of a prefix for numbers.

Elysia, thank you. As I read your reply I was banging my head and saying "I knew that." All too often I get caught in the act of thinking content or value rather than pointer. This probably comes from the many years (most of my first ten years of programming) of writing in REXX which is so forgiving that you rarely use pointers.

Thanks.

Now, I'll have to figure out a good way to phrase my questions on win32's WriteFile function when using structures. I remember quite quickly concluding "this is nuts" and going right back to fopen and fwrite and fputs and the likes.

Dale L

But fopen/fwrite are C standard functions, WriteFile is not. You would be sacrificing portability.

Elysia, agreed. Hence my copies of K&R and Petzold are pretty much resident on my desk. Going through Petzold it is obvious that he is trying to remain win32 compliant. So one day I get to playing with WriteFile and seems that things blow up.

For example, from the structure in the code above. Lets say I have

NOTEINFO note[NumberOfNotes] ;

NumberOfNotes may be pretty much any number: lets say 25.

Now, I want to drop all 25 of the notes to file using WriteFile. I remember playing with it for quite a while and got unexpected results. Probably forgot about pointers again.

It would seem that there would be a nice quick and easy one liner to write the 25 notes to file. Any chance you have that one liner on the tip of your tongue?

Dale L

For C:
fwrite(note, sizeof(note), 1, f);

Only works if note is defined in the same function where you write the file. Otherwise you're going to have to do something like:
fwrite(note, sizeof(*note) * num, 1, f);

Elysia, yes. Thanks. I've got it all working in standard C routines. I was curious as to how it would be done with win32's WriteFile. I pulled the following line from some old code that didn't work:

WriteFile (hFile, note[ctr], sizeof(note[ctr]), &dwBytesWritten, NULL) ;

Don't knock yourself out on this one. Just toying with things that I've done in the past in, perhaps, far less than professional style.

Dale L

Try this instead:

Code:

---

WriteFile (hFile, note, sizeof(note), &dwBytesWritten, NULL) ;

---

It's basically the same as the C code.

Bear in mind that writing a struct that contains pointers to a binary file is pretty useless, because if when you load the data next time, the pointers will not be valid [1]. You need to either:
1. Use fixed length strings in your note data.
This is by far the easiest to implement.

2. Write the data out in a different way (so you store the string pointed to by the pointer, rather than the pointer itself).
This is trickier, but also more flexible as it copes with any length body/extra, as you have to know how much data you have to read, so you first read the nSize, then allocate nSize bytes for nBody, and read nSize bytes into your new pointer, and finally you need to know the length of nExtra and do the same thing there.

[1] Of course, if you don't de-allocate the memory (or exit the application), it will still be valid if when you read it back - but the point of storing something is usually that you want to exit the application and then read it back when you start the application again.

--
Mats

Thanks for the input. While the WriteFile line I gave was not quite a real life example, your comments certainly did help shake the cobwebs and clear things up. Thanks.

Now, if you don't mind, another concern.

Somewhere I've read that after doing a malloc (or alloc) it is good to set the variable value to NULL such as:

Code:

---

note[ctr].nBody = malloc (300 * sizeof( char ) ) ;
note[ctr].nBody = NULL ;

---

Okay, that kinda makes sense. As I understand, it would leave the var pointing to nothing. Would this be done to help in the area of memory leaks? Is this a good practice? Would I really want to bother setting to NULL if I'm actually going to fill the nBody immediatly after the malloc?

Thoughts are appreciated.

Dale L

Yes, it's a good practice. However, you must free it first, otherwise you get a memory leak.
It allows you to see if it's a valid pointer or not, which is why it's such a good thing.
Don't do it right after malloc (your memory you just allocated will be lost!).

Let me make sure I understand: setting it to NULL will actually defeat the malloc? So if I set nBody to NULL before doing anything with it I would be defeating the malloc?

In other words, I would want to first do malloc on nBody and then not until after done using nBody, I would free it and then set it to NULL?

Dale L

Yes to first and second.

>> Somehow I've managed to program for a LONG time and never bothered to understand some things. All too often I've done "just make it work" programming.

it's pointless to program in a language you don't understand so well. you will spend a lot of time debugging problems that would have never been an issue had you just consulted a good C programming book.

>> Okay, that kinda makes sense. As I understand, it would leave the var pointing to nothing. Would this be done to help in the area of memory leaks? Is this a good practice? Would I really want to bother setting to NULL if I'm actually going to fill the nBody immediatly after the malloc?

seriously.

Code:

---

note[ctr].nBody = malloc (300 * sizeof( char ) ) ;

---

Now do something with your 300 chars.

Then when you're done, you do this.

Code:

---

free( note[ctr].nBody );
note[ctr].nBody = NULL ;

---

Thanks again for everyone's input. I dug around and found the old code from which this originates. Mind you, this works but it doesn't feel right and I'm not sure of one particular part. I stripped the code to keep it small for posting.

First, here is a data file named ttt.dat

05/16/08 22:31 aaaa
05/16/08 22:32 bbb
05/16/08 22:33 asdfasdf

Each line needs to be manipulated so I read each line as one string and then start manipulation. Here is the code:

Code:

---

#include <stdio.h>
#include <string.h>
#include <malloc.h>

#define NumberOfNotes 25      /* The number of notes the program will allow    */

typedef struct {                        /* structure of note        */
        char nBody[20];                /* text of note                */
        char nExtra[300];                /* area for extra info such as date string        */
} NOTEINFO;

NOTEINFO *note[NumberOfNotes] ;

main ()
{
        FILE *dataStream ;
        int ctr ;                                                /* utility counter */
        int finis = 0 ;                                /* boolen for testing */
        char noteContent[300] = "" ;                /* string for manipulation of file data */

        for( ctr = 0; ctr < NumberOfNotes; ctr++ )        /* loop through number of notes */
        {        note[ctr] = (NOTEINFO *)malloc( sizeof( NOTEINFO ) ) ;        /* allocate memory */
                if( note[ctr] == NULL )
                {        printf( "Error allocating." ) ;        /* print error */
                        finis = 1 ;
                }
        }

        dataStream = fopen( "ttt.dat", "r" ) ;                /* open file for reading */
        if( dataStream == NULL )                                /* test opening */
        {        printf( "Error opening file." ) ;                /* print error message */
                finis = 1 ;
        }

        ctr = 0;
        while( !finis )
        {
                if( fgets( noteContent, 300, dataStream ) == NULL )        /* read the string */
                        finis = 1 ;                                        /* set boolean for end of read */
                else
                {        strncpy( note[ctr]->nExtra, noteContent, 14 ) ;        /* copy first 14 chars to nExtra */
                        printf( note[ctr]->nExtra ) ;                /* print nExtra */
                        printf( "\n" ) ;
                        strncpy( note[ctr]->nBody, &noteContent[19], strlen( noteContent) - 1 ) ;
                        printf( note[ctr]->nBody ) ;                /* print nBody */
                }
                ctr++ ;
        }
        fclose( dataStream );

        for( ctr = 0; ctr < NumberOfNotes; ctr++ )        /* loop through all note variables */
                free( note[ctr] ) ;                                        /* free the memory */

        return( 0 ) ;
}

---

First I'm confused with the strncpy(). It is used twice: first to grab the date info via:

Code:

---

strncpy( note[ctr]->nExtra, noteContent, 14 ) ;

---

And then for the note body:

Code:

---

strncpy( note[ctr]->nBody, &noteContent[19], strlen( noteContent) - 1 ) ;

---

As I understand, the second param to strncpy is a string pointer. Why do I need the "&" in the second strncpy but not the first?

Second, the whole things just looks sloppy. I'm open for suggestions on cleaner code.

Thanks in advance.

Dale L

You could have written the first as
strncpy( note[ctr]->nExtra, &noteContent[0], 14 ) ;


> #include <malloc.h>
Must be very old code then. The place where allocation functions are prototyped is stdlib.h

> note[ctr] = (NOTEINFO *)malloc( sizeof( NOTEINFO ) ) ;
In ANSI-C, there is no need for the cast. Though if you're still compiling this old code on an old compiler, it might still be.

There is actually a bug in the way allocated memory is used.
The strncpy may not actually copy a \0 to make the result a proper C string, and malloc doesn't guarantee to clear memory either. So the result may be you see some (perhaps quite a lot) of random characters at the end of your line.


> printf( note[ctr]->nExtra ) ;
NEVER do this - that is pass user input as the format string to printf.
http://en.wikipedia.org/wiki/Format_string_attack

Thanks for the info Salem. Wish I had known about the malloc situation years ago. I can remember pulling my hair out over the unexpected characters.

I'm thinking now (beware!)

What if you took an old C program that uses malloc and rename it to a cpp file, then you take the mallocs and use "new" instead? If memory serves, some of the header files could also be removed.

It would seem that doing so would clean up the allocation and string problems. Do you know?

Thoughts welcome.

Dale L

New is identical to malloc, but it takes into account C++'s new features, such as classes, and therefore also constructors and destructors. Otherwise there's little difference between the two.
So in itself, it wouldn't help anything. RAII could help with allocation/deallocation, but that's a different subject.

Agreed. (But then I DID warn you when I started thinking.)

To ensure all comes up clean I suppose following the malloc with a memset would work.

Dale L

Or you could use calloc.

Quote:

---

Originally Posted by robwhit

Or you could use calloc.

---

Yes, it's probably more efficient than a combination of malloc() and memset(), if nothing else because it's only one call in your code so it makes the code a tiny bit shorter.

--
Mats