I am a novice in C. I am trying to assign a value from a file to a pointer as below. However I am gettign a seg fault. Can somebody explain ?

int i1;
char *glblclrtab;
...

for (int i=0; i<20; i++)
{ while ( (i1= fgetc(ipFile))==NULL);
*(glblclrtab+i)= char (i1);
...
}

The pointer assignment in causing segmentation fault.

PLZ HELP
Thanks in Advance

What's glblclrtab pointing to? It'll have a NULL value or a garbage value which is causing segmentation fault. After you make it point to a character array, you can use *(glblclrtab+i)= (char) i1; or glblclrtab[i]= (char) i1; for simplicity.

By the way, use code tags while posting your code.

char *glblclrtab; by itself will not allocate any memory to be use.
malloc() must be invoked to set apart as much memory as needed to hold the data you intend to assigned (point) to it.

Thanks, I had not pionted "glblclrtab". This is just an array to store the color values from file.

char *glblclrtab; by itself will not allocate any memory to be use.
malloc() must be invoked to set apart as much memory as needed to hold the data you intend to assigned (point) to it.

I am using malloc now, however when I read the data back it gives garbage data. Following is the part of the code:

...
...
glblclrtab= (char *)malloc(sizeof(3*tmp));

for (int i=0; i<(3*tmp); i++)
{
if (tmp2==3)tmp2=0;
i1=fgetc( ipFile );
if (tmp2==0) {*(glblclrtab + clrcnt) = (char)i1;}
else if (tmp2==1) {*(glblclrtab + tmp +clrcnt) = (char)i1;}
else if (tmp2==2) {*(glblclrtab + 2*tmp + clrcnt)= (char)i1;}

tmp2++;
}
free (glblclrtab);


When I use free as shown above, program dumps backtrace memory range in the end and aborts.

Thanks in advance

Thanks, I had not pionted "glblclrtab". This is just an array to store the color values from file.

You want 'glblclrtab' to be an array, but C compiler have no idea what your intentions are. So allocate memory before storing.
As arrays have predefined space in the memory, we do not allocate memory for them but in your case its need to be done manually.

You have no idea where 'glblclrtab' is pointing in the memory space. and neither anyone in the world have.

That is is reason I used the malloc before storing in the for() statement. However, I cannot use array as the size is not predefined. I have to use it on the fly. I had assigned glblclrab as char pointer as:

char *glblclrtab;

I do not know what is the right way of using the pointer here.

  1. Please, USE code tag for your snippets properly:

    [code=c]
    your source texts
    [/code]
    
  2. You don't understand operator sizeof functionality. See what happens in your (incorrect) memory allocation:

    glblclrtab= (char *)malloc(sizeof(3*tmp));
    

    The malloc functions allocates sizeof(3*tmp) bytes.

    The C Standard:

    The sizeof operator yields the size (in bytes) of its operand, which may be an expression or the parenthesized name of a type. The size is determined from the type of the operand.

    Suppose the variable tmp is int, integer literal 3 is int by definition. Therefore a type of an expression tmp*3 is int. Now sizeof(tmp*3) == sizeof(int). For example, you allocate 4 bytes only in 32-bit environment.

    If you want allocate tmp*3 bytes, write

    glblclrtab= malloc(3*tmp); 
    /* No need to convert void* to char* in C explicitly */
    
  3. What's a horrible name - glblclrtab! Try to pronounce it ;)...

Edited 3 Years Ago by Dani: Formatting fixed

I made the changes as:

glblclrtab= malloc(3*tmp);

I could eliminate the error that was coming from free(). The program nolonger aborts.

However, I still do not get the values as stored.

I made the changes as:

char *glblclrtab;
 glblclrtab= malloc(3*tmp); 
for (int i=0; i<(3*tmp); i++)
{ // effort to sort the data from file, size unknown (tmp), I know when I parse the file
if (tmp2==3)tmp2=0;
i1=fgetc( ipFile );
if (tmp2==0) {*(glblclrtab + clrcnt) = (char)i1;}
else if (tmp2==1) {*(glblclrtab + tmp +clrcnt) = (char)i1;}
else if (tmp2==2) {*(glblclrtab + 2*tmp + clrcnt)= (char)i1;}
tmp2++;
}

for (int i=0; i<(tmp); i++)
{ printf("%d::%d, %d, %d \n",i,*(glblclrtab+i),*(glblclrtab + tmp + i),*(glblclrtab+2*tmp+i));
// [U][B]I receive garbage values here[/B][/U]
}
free (glblclrtab);  // program nolonger aborts

I could eliminate the error that was coming from free(). The program nolonger aborts.

However, I still do not get the values as stored.

If I read the code right, the intent is to split up color table values?

bytes in the file come as r,g,b,r,g,b...

You want the array to contain r,r,r...g,g,g...b,b,b...

Is that close?

if so, what do you think of:

char *glblclrtab;
glblclrtab= malloc(3*tmp); 

for (int i=0; i < tmp; i++)
{ 
    glblclrtab[i] = fgetc( ipFile);
    glblclrtab[i + tmp] = fgetc( ipFile);
    glblclrtab[i + 2 * tmp] = fgetc( ipFile);
}

for (int i = 0; i < tmp; i++)
{ 
    printf("%d::%d, %d, %d \n",
        i,glblclrtab[i], glblclrtab[tmp + i],glblclrtab[2*tmp+i]);
}

free (glblclrtab);  // program nolonger aborts

note that *(glblclrtab + i) is functionally equivalent to glblclrtab[i] but the second is way easier to read.


What was the variable clrcnt in your code?

clrcnt is just a variable to segregate the RGB values. However, what I am looking at is that I am not retreiving the same values as stored. Is there a flaw in code ? I could have used array if I had know the array size beforehand. However, I get the size only when I read the file, so to save space, malloc was seemingly a good solution to me.

malloc is the right solution, but you can treat a pointer to allocated memory like an array of the same type. As I said, the expressions are interchangeable, I just prefer the second.

isn't tmp the number of bytes between sections?

the code never updated clrcnt so if it was zero, all data is stored in the first entry in the table. If it was non-zero, all the data is stored at that index.

This would work

int clrcnt = 0;
for (int i=0; i<(3*tmp); i++)
{ 
    if (tmp2==3) 
    {
        tmp2=0; 
        clrcnt++;
    }
    i1=fgetc( ipFile );
    if (tmp2==0) {
        *(glblclrtab + clrcnt) = (char)i1;
    } 
    else if (tmp2==1) {
        *(glblclrtab + tmp +clrcnt) = (char)i1;
    } 
    else if (tmp2==2) {
        *(glblclrtab + 2*tmp + clrcnt)= (char)i1;
    }
    tmp2++;
}

This is cleaner:

int clrcnt = 0;
for (int i=0; i<tmp; i++)
{ 
    i1=fgetc( ipFile );
    *(glblclrtab + clrcnt) = (char)i1;
    i1=fgetc( ipFile );
    *(glblclrtab + tmp +clrcnt) = (char)i1;
    i1=fgetc( ipFile );
    *(glblclrtab + 2*tmp + clrcnt)= (char)i1;
    clrcnt++;
}

now that i and clrcnt always have the same value:

for (int clrcnt=0; clrcnt<tmp; clrcnt++)
{ 
    i1=fgetc( ipFile );
    *(glblclrtab + clrcnt) = (char)i1;
    i1=fgetc( ipFile );
    *(glblclrtab + tmp +clrcnt) = (char)i1;
    i1=fgetc( ipFile );
    *(glblclrtab + 2*tmp + clrcnt)= (char)i1;
}

note that *(glblclrtab + i) is functionally equivalent to glblclrtab but the second is way easier to read.

Not entirely true, it is possible that the array was broken up in memory (to save space, or maybe just to piss programmers off). The first approach will not account for that, the second will. Plus, if you used the first, you have to be careful of accounting for the size of each array element.

Not entirely true, it is possible that the array was broken up in memory

Does that ever actually happen? I thought arrays are always assigned contiguous blocks of memory.

> I thought arrays are always assigned contiguous blocks of memory.
They are.
As are all blocks of memory returned by malloc / calloc / realloc

The link posted by death_oclock in the previous post is just a load of rubbish.

They seem to think that because the OS might map parts of the array to different pages of memory, that it has any effect on a C program. It DOESN'T. The virtual address is contiguous, and that's all that matters.

Comments
Thanks! :)
This article has been dead for over six months. Start a new discussion instead.