tags:

views:

402

answers:

3

Hi, I am having trouble figuring out the reason why my .c code is having trouble allocating ~250K of memory. Here is the allocation code:

struct IMAGE {
    int width, height, maxval;
    char **data;
};

void raiseError(char *msg)
{
    printf("%s", msg);
    getch();
    exit(1);
}

//...

IMAGE readPGM()
{
    IMAGE image;
    image.data = (char **) malloc(sizeof(char)*image.height);

    //..

    for (i=0; i<image.height; i++) {
        image.data[i] = (char *) malloc(sizeof(char)*image.width);
        if (image.data[i]=='\0') {
            printf("%d\n", i);
            raiseError("Not enough memory!..");
        }
    }

    //..
}

//..

The program exits when i=116. image.width and image.height equals to 500 here, so I want 500x500=250000 bytes to be allocated here. But 116x500 = 58000 bytes are being allocated at maximum. So, is there something that limits it? Is there something wrong with my code? I am posting the full source below, just in case if it is necesarry. The idea is to read a PGM file into the structure IMAGE, process it and rewrite it in another file. As you can tell, it is not complete yet because I couldn't figure out a way to allocate more memory.

#include<stdio.h>
#include<conio.h>
#include<ctype.h>
#include<stdlib.h>
#include<string.h>
#include<ctype.h>
#include<alloc.h>
struct IMAGE {
    int width, height, maxval;
    char **data;
};

void raiseError(char *msg)
{
    printf("%s", msg);
    getch();
    exit(1);
}

char *toString(int num)
{
    char sign = 0;
    if (num<0) {
        sign = -1;
        num*=-1;
    }
    int numLen = 1;
    if (sign<0) {
        numLen++;
    }
    int tmpNum = num;
    while (tmpNum>9) {
        tmpNum /= 10;
        numLen++;
    }
    char *result = (char *)malloc(sizeof(char)*(numLen+1));
    result[numLen] = '\0';
    char ch;
    while (num>9) {
        ch = (num%10)+'0';
        num /= 10;
        result[numLen-1] = ch;
        numLen--;
    }
    result[numLen-1] = num + '0';

    if (sign<0)
        result[0] = '-';
    return result;
}

int toInteger(char *line)
{
    int i=strlen(line)-1;
    int factor = 1;
    int result = 0;
    while (i>=0) {
        result += factor*(line[i]-'0');
        factor *= 10;
        i--;
    }
    return result;
}

char *getNewParam(FILE *fp)
{
    char ch = 'X';
    char *newParam;
    newParam = (char*) malloc(1);
    newParam[0] = '\0';
    int paramSize = 0;
    while (!isspace(ch)) {
        ch = fgetc(fp);
        if (!isspace(ch)) {
            if (ch=='#') {
                while (fgetc(fp)!='\n');
                continue;
            }
            paramSize++;
            newParam = (char *) realloc(newParam, paramSize+1);
            newParam[paramSize-1] = ch;
        }
    }
    newParam[paramSize] = '\0';
    return newParam;
}

IMAGE readPGM()
{
    FILE *fp;
    IMAGE image;
    //Open the file.
    fp = fopen("seeds2.pgm","r+b");
    if (fp=='\0')
        raiseError("File could not be opened!..");

    //Check if it is a raw PGM(P5)
    char *line;
    line = getNewParam(fp);
    if (strcmp(line, "P5")!=0)
        raiseError("File is not a valid raw PGM(P5)");
    int paramCount = 0;
    int *pgmParams;
    pgmParams = (int *)malloc(sizeof(int)*3);
    while (paramCount<3) {
        line = getNewParam(fp);
        pgmParams[paramCount++] = toInteger(line);
    }
    int pixelSize;
    if (pgmParams[2]>255)
        pixelSize = 2;
    else
        pixelSize = 1;

    image.width =pgmParams[0];
    image.height =pgmParams[1];
    image.maxval =pgmParams[2];
    free(pgmParams);
    image.data = (char **) malloc(sizeof(char)*image.height);
    int i,j;
    long sum = 0;
    for (i=0; i<image.height; i++) {
        image.data[i] = (char *) malloc(sizeof(char)*image.width);
        sum += sizeof(char)*image.width;
        if (image.data[i]=='\0') {
            printf("%d\n", i);
            raiseError("Not enough memory!..");

        }
    }
    for (i=0; i<image.height; i++) {
        for (j=0; j<image.width; j++) {
            fread(&image.data[i][j], sizeof(char), image.width, fp);
        }
    }
    fclose(fp);

    return image;
}

void savePGM(IMAGE image)
{
    FILE *fp = fopen("yeni.pgm", "w+b");
    fprintf(fp, "P5\n%s\n%s\n%s\n",
     toString(image.width), toString(image.height), toString(image.maxval));
    int i,j;
    for (i=0; i<image.height; i++) {
        for (j=0; j<image.width; j++) {
            fwrite(&image.data[i][j], sizeof(char), 1, fp);
        }
    }
    fclose(fp);
}

int main()
{
    clrscr();
    IMAGE image = readPGM();
    //process
    savePGM(image);
    getch();
    return 0;
}
+11  A: 

I'm not going to read all that code -- this is a prime example of where you should give a minimal example to reproduce the problem -- but this line:

image.data = (char **) malloc(sizeof(char)*image.height);

is incorrect. It should have sizeof(char*). The statement as written should return a char* but you are casting it to char**.

FWIW, on my system, sizeof(char) returns 1 and sizeof(char*) returns 4, because a char is a single byte, and a char* is a pointer (aka a single word of 32 bits). So you actually allocated 1/4 of what you probably intended to allocate.

Mark Rushakoff
Also, sizeof(char) is always 1, by definition. There's no need to ever write sizeof(char) * X.
Charles Salvia
Moreover, you may corrupting your pointers when executing `image.data[i] = (char *) malloc(sizeof(char)*image.width);` as you may overwrite the end of the pointer returned by malloc.Another problem is that `image.data[i]=='\0'`tests only the first byte of that pointer.
swegi
@swegi: `image.data[i] == '\0'` is actually ok, but it should be rewritten. `'\0'` is equivalent to `0`, which is equivalent to `NULL`, so it's the same as testing `image.data[i] == NULL`.
Adam Rosenfield
@Mark: I have corrected that mistake of mine, but now it exits when i=113, so it didn't help. And by the way, when I run the .EXE without the IDE, it allocates the memory without returning any '\0'. But when I open the file I created, I can tell that there was some corruption with the pointers like swegi pointed out, because it's not the data I want, it's some other memory block which happens to be there, probably. I'll look into it in the morning though, thanks for all the help.
Erkan Haspulat
@AdamR, you're right that the effect is the same but I tend to use NULL exclusively for pointers. I'll use '\0' for character 0 since it seems to convey the intent better. Interestingly (to me) though, 0 is used for integers but I don't follow through and use 0L or 0.0 for longs and floating point (or U/LL/ULL for unsigned and the long longs).
paxdiablo
Thank you everyone for your help, I'll be using a new compiler and I beleive this will solve the problem.
Erkan Haspulat
+1  A: 

If you print out the value that you are passing to malloc, you will find out how much memory you are really asking for.

gbarry
+11  A: 

The answer to your question is in the comment you added to it. You are using an (antique) 16 bit x86 real mode compiler. A 16 bit virtual machine can address only 1Mb of memory in total, only 640Kb of which is normally accessible to programs, and is shared with the OS.

[edit in response to paxdiablo's comment]

Further to those restrictions the segmented addressing architecture also gives rise to a number of memory models where as little as 64kb may be the limit for specific memory areas.

Also the argument to malloc() has type size_t, which may only be 16-bit in this case - you should check. I recall using a variant called halloc() for large allocations. But short of using a DOS extender, there is still a 640kb limit.

[end edit]

There are a number of far better and free modern 32-bit compilers available. Dump the antique. I suggest VC++ 2008 Express Edition

Apart from that strictly:

if(image.data[i]=='\0')

should be

if(image.data[i]==0)

or

if(image.data[i]==NULL)

As it happens it will work in either case, but technically you are testing for a null-pointer not a NUL character.

Clifford
+1 but with one proviso. Turbo C (IIRC) had memory models, where you could have 64K for code/data/stack (tiny), and then various other options up to separate 64K each for code, data and stack (large). But I think there was a huge memory model which allowed multiple 64K segments for data. I cannot recall whether *each* data structure was still limited to 64K though. Caveat: I haven't used Borland for *many* years, my memory may be faulty. But Clifford is correct, get yourself a more modern compiler. VC Express, gcc, CodeBlocks, none of them cost any money.
paxdiablo
I guess that seems to be the problem, I should have noticed that when I saw I could allocate 58000 at maxiumum. And @paxdiablo, I have tried to use the huge model, but it looks like each data structure is still limited to 64K.
Erkan Haspulat
Does the Turbo C library support halloc()? The problem is probably that size_t is 16bit, halloc() accepts a 32bit argument. However I only ever used this on Microsoft's DOS compilers, so it may not be supported or may have a different name.
Clifford