Error on using malloc()

[Nobun]

Preface

I never use pointers since now.
I'm trying to develop a graphical engine (using allegro 4.22) and my 1st task is to store in memory a pack of PCX images.
This task reads a binary file that stores all PCX and some other informations.
So, seeing the fact I don't know HOW MANY IMAGES I have to store nor the length of any image, I thinking about using pointers.

BITMAP ** pcxstore; //note: BITMAP is defined into Allegro library
//... other code ...
pcxstore = (BITMAP **) malloc (sizeof (BITMAP *) * total_images);
//...other code ...
for(i=0;i<total_images;i++) {
  //...other code ...
  while(!key[KEY_ESC]);
  pcxstore[i] = (BITMAP *) malloc (pcx_length); //Here is the error. Why?
  while(!key[KEY_ESC]);
  //...other code...
}

notes:

total_images (unsigned short) contains the number of total images to be stored

pcx_length (unsigned long) is a value equal to size in bytes of the single PCX I try to store (so every times changes)

while(!key[KEY_ESC]); // this instruction waits untill you press the ESC key. It uses "key[KEY_ESC]" (defined into Allegro)

-----

What happens:

After the 1st pressing of ESC (used like a "breakpoint") the program ends (it is unexpected) so the error should be where I said (probably I used wrongly malloc).

I used this trick (with the ESC waiting as a "breakpoint)" seeing the fact that some of the final lines of my code are not executed... I tried to find the error.

Sorry for my bad english.

---

Language: C++

Compiler:

G++ 3.4.5 - MingW 5.1.4 (system: windows XP)

I also installed and I also using allegro library (allegro 4.22)

-----

[helios]

Are you sure that's how the library defines a BITMAP should be created? I don't think a BITMAP should simply be an array of bytes.
Also, use new instead of malloc(). It looks cleaner.

[Nobun]

1st of all thank for your reply...
However I have to say that, searching a lot, I find that the unexpected closing is not directly related to that instruction I said, but it was related with a bad setting of reading the block of PCX "file" from binary source (in.read(etc))...

However you make the point when you said that the BITMAP is not a simply array of bytes. I verify that BITMAP is a little complex structure (so it is better to use allegro functions instead of this "manual" - and wrong - method).

But now I have a problem. I can save any block as a temp file and load it with the "load_bitmap(const char * filename, RGB *pal)" Allegro function and then remove it, but I think is not a good way (seeing the fact my program will load MANY images and have to load binary pack MANY times)... this method can be dangerous for HD of users, i think.

So I should find a way to use the informations into the variable char * buffer (that temporary memorize the PCX "file" block) and stores into BITMAP.

(From Allegro documentation)

typedef struct BITMAP

int w, h; - size of the bitmap in pixels
int clip; - non-zero if clipping is turned on
int cl, cr, ct, cb; - clip rectangle left, right, top,
and bottom
unsigned char *line[]; - pointers to the start of each line

There is some other stuff in the structure as well, but it is liable to
change and you shouldn't use anything except the above. The `w' and `h'
fields can be used to obtain the size of an existing bitmap:

bmp = load_bitmap("file.bmp", pal);
allegro_message("Bitmap size: (%dx%d)\n", bmp->w, bmp->h);

Seeing the fact that Allegro docs suggest to use Allegro functions to "declare" a BITMAP (and not a direct use like I tryed before by myself) I don't know how to avoid problem.

the "file" functions requires all to load a real file from filename so I cannot use instructions like "load_bitmap()" and similar, seeing the fact I cannot use the "buffer" address point (it is not a real file, but an array with the bytes of the PCX "file" block).

Perhaps the solution can be found into Allegro function "create_bitmap"

BITMAP *create_bitmap(int width, int height);
Creates a memory bitmap sized width by height. The bitmap will have
clipping turned on, and the clipping rectangle set to the full size of the
bitmap. The image memory will not be cleared, so it will probably contain
garbage: you should clear the bitmap before using it. This routine always
uses the global pixel format, as specified by calling set_color_depth().
The minimum height of the BITMAP must be 1 and width can't be negative.
Example:

/* Create a 10 pixel tall bitmap, as wide as the screen. */
BITMAP *bmp = create_bitmap(SCREEN_W, 10);
if (!bmp)
abort_on_error("Couldn't create bitmap!");
/* Use the bitmap. */
...
/* Destroy it when we don't need it any more. */
destroy_bitmap(bmp);

Returns a pointer to the created bitmap, or NULL if the bitmap could not
be created. Remember to free this bitmap later to avoid memory leaks.

Any other ideas?

---------

Final note:

Also, use new instead of malloc(). It looks cleaner

I find malloc() more customizable than the "new" operator. In my opinion the fact you can decide the size in bytes of memory allocation makes it better than new (also it needs more attention).

I prefer use new operator in other situations like changing and resetting dimension of "buffer"

char * buffer;
buffer = new char[4];
//When I need 4 bytes storer
delete [] buffer;
buffer = new char[100];
//When I need 100 bytes storer
delete [] buffer;

[helios]

I find malloc() more customizable than the "new" operator. In my opinion the fact you can decide the size in bytes of memory allocation makes it better than new (also it needs more attention).

You do realize that a char is (as far as I can remember) ALWAYS 1 byte long. Actually, the real reason to use malloc() over new is optimization (malloc() is from 10% to 50% faster than new, depending on the size of memory you're trying to allocate).

So, what's the problem? You need to load many (no idea how much "many" is to you. To me it would be over 10000) images and want to reduce HDD usage? Well, unless the same image is going to be loaded more that once, you don't have much of a choice. If you are going to load the same image more than once, you could use a memory cache. Keep in mind that you'll be moving the strain from HDD access to RAM usage.
Don't worry. There's nothing "dangerous" about accessing the HDD. Since you'll be processing the files between reads, you won't be able to use more than... 5 MB/s. You only need to worry if you're using a considerable amount of bandwidth (usually more than 50 or 100 MB/s) for a prolonged period of time (I suppose it should be somewhere between a few hours and a few days). But those numbers only come up during tasks such as copying entire disks, and they don't run for more than a few hours.

[Nobun]

You do realize that a char is (as far as I can remember) ALWAYS 1 byte long

Yes, I know it. But I can't understand what you are trying to say, I'm sorry.
If you are talking about my example of usage of new and delete you can see that I use a pointer that points to an array of chars (or, better, an array of bytes, seeing the fact I use it as a binary data storer).
If you are talking about the (void *) malloc function I don't also understand what you are meaning.
Probably I lost a step of your reply, sorry again.

So, what's the problem? You need to load many (no idea how much "many" is to you. To me it would be over 10000) images and want to reduce HDD usage? Well, unless the same image is going to be loaded more that once, you don't have much of a choice. If you are going to load the same image more than once, you could use a memory cache. Keep in mind that you'll be moving the strain from HDD access to RAM usage.

10,000 images is more or less realistic. I try to emulate a customizable fighting game. I have to load up to 8 fighter for every match, a stage, a lifebar and spark effects.
A single char can have from 200 to 1000 or more images.
A stage, usually, does not have too much images.
lifebar, and sparks (also fonts, perhaps) have other images to be memorized.

The problem is that I cannot verify if a PCX "file" is used more than once, seeing the fact I have not to parse a real PCX file, but a binary file that it is a binary "pack" of PCX so I cannot load it normally (infact what I try to do in this moment is the "pack" decoder that has to load into memory all images of all "packs" loaded).
Seeing the fact that I have to load from 2 to 8 chars at the same time (every char has its own binary file that stores all own PCX) you can understand that I have to decode a VERY HUGE quantity of PCX.

If I create-destroy 1,000 temp images (can be MORE that ONLY 1,000 images)
every time I need to load images (more or less every single match) I can risk to corrupt HD becouse it is not a good way to write and cancel repeatly into the HD (the HD can be fisically ruined by time, I think).
So this is the reason that it is not a good idea to create temp files only to be able to use the function "load_bitmap()" that requires a fisical PCX file and cannot be used with a PCX "file" contained into a binary file as a block of bytes (and stored into char * buffer).

If I am wrong say to me (i am here to learn ;) )

won't be able to use more than... 5 MB/s. You only need to worry if you're using a considerable amount of bandwidth (usually more than 50 or 100 MB/s) for a prolonged period of time (I suppose it should be somewhere between a few hours and a few days). But those numbers only come up during tasks such as copying entire disks, and they don't run for more than a few hours

I didn't know it, thank for information (I never know since now the velocity of BUS). However it is not a great problem. Usually the size of single pcx block is from 2 to 10 Kb as I can see so far.

[helios]

Yes, I know it. But I can't understand what you are trying to say, I'm sorry.
If you are talking about my example of usage of new and delete you can see that I use a pointer that points to an array of chars (or, better, an array of bytes, seeing the fact I use it as a binary data storer).
If you are talking about the (void *) malloc function I don't also understand what you are meaning.
Probably I lost a step of your reply, sorry again.

What I meant was that you can also use as many bytes as you want with new. Let's say that we have a structure S that uses n bytes. If you wanted to allocate this space the hard way, you would do S *a=(S *)new char[sizeof(S)]. This is, of course, as retarded as doing long *b=(long *)new char[sizeof(long)]. So honestly, I don't know what advange other than a small speed increase you're getting from using malloc() over new.

Now, back to the problem.

Okay, now I see what you're trying to do. You load the archive to memory, then you dump the images to disk as temp files and load them back to memory, only to delete them temporary files afterwards. Let me say this now: you're doing it wrong. Now that that's behinds us, let's see what you should be doing.

Are you sure there isn't a way to load an image from memory as if it was a file? I'm pretty sure there should be one. Possibly a structure that abstracts the source of data. If there's ABSOLUTELY NO WAY to do this, Allegro is not worth the disk space it uses.

[Nobun]

Yes I know that it is a wrong way-to-do... so I was asking for help to find a way to load an image from memory (seeing the fact that allegro doesn't support directly this case). Seeing the fact I am too novice to try to write by myself a routine like that, I was hoping to find an help to write that type of function.

Luckly I found a code to use for solving my problem (a function made by someone that loads a bitmap from memory thinked to be used in Allegro). I did only a little change to a better adapting to my own code.

However thank you very much for your time :)