CODEWITHSUNDEEP
cpointers
Danial
Danial

Reputation: 612

pointer of pointer of char in c, assignment crashes

I have a pointer of pointer to store lines I read from a file;

char **lines;

And I'm assigning them like this :

line_no=0;
*(&lines[line_no++])=buffer;

But it crashes why ?

According to my logic the & should give the pointer of zeroth index, then *var=value, that's how to store value in pointer. Isn't it ?

Here is my current complete code :


void read_file(char const *name,int len)
{
    int line_no=0;
    FILE* file;
    int buffer_length = 1024;
    char buffer[buffer_length];

    file = fopen(name, "r");

    while(fgets(buffer, buffer_length, file)) {
        printf("---%s", buffer);
        ++line_no;
        if(line_no==0)
        {
            lines = (char**)malloc(sizeof(*lines) * line_no);
        }
        else
        {
            lines = (char**)realloc(lines,sizeof(*lines) * line_no);
        }
        lines[line_no-1] = (char*)malloc(sizeof(buffer));
        lines[line_no-1]=buffer;
        printf("-------%s--------\n", *lines[line_no-1]);
    }

    fclose(file);
}

Upvotes: 0

Views: 93

Answers (2)

Jorenar
Jorenar

Reputation: 2884

You have just a pointer, nothing more. You need to allocate memory using malloc().

Actually, you need first to allocate memory for pointers, then allocate memory for strings.

N lines, each M characters long:

char** lines = malloc(sizeof(*lines) * N);
for (int i = 0; i < N; ++i) {
    lines[i] = malloc(sizeof(*(lines[i])) * M);
}

You are also taking an address and then immediately dereference it - something like*(&foo) makes little to no sense.

For updated code

Oh, there is so much wrong with that code...

  1. You need to include stdlib.h to use malloc()
  2. lines is undeclared. The char** lines is missing before loop
  3. if in loop checks whether line_no is 0. If it is, then it allocates lines. The problem is, variable line_no is 0 - sizeof(*lines) times 0 is still zero. It allocates no memory.
  4. But! There is ++line_no at the beginning of the loop, therefore line_no is never 0, so malloc() isn't called at all.
  5. lines[line_no-1] = buffer; - it doesn't copy from buffer to lines[line_no-1], it just assigns pointers. To copy strings in C you need to use strcpy()
  6. fgets() adds new line character at the end of buffer - you probably want to remove it: buffer[strcspn(buffer, "\n")] = '\0';
  7. Argument len is never used.
  8. char buffer[buffer_length]; - don't use VLA
  9. It would be better to increment line_no at the end of the loop instead of constantly calculating line_no-1
  10. In C, casting result of malloc() isn't mandatory
  11. There is no check, if opening file failed
  12. You aren't freeing the memory

Considering all of this, I quickly "corrected" it to such state:

void read_file(char const* name)
{
    FILE* file = fopen(name, "r");
    if (file == NULL) {
        return;
    }

    int buffer_length = 1024;
    char buffer[1024];

    char** lines = malloc(0);

    int line_no = 0;
    while (fgets(buffer, buffer_length, file)) {
        buffer[strcspn(buffer, "\n")] = '\0';
        printf("---%s\n", buffer);
        lines = realloc(lines, sizeof (*lines) * (line_no+1));
        lines[line_no] = malloc(sizeof (*lines[line_no]) * buffer_length);
        strcpy(lines[line_no], buffer);
        printf("-------%s--------\n", lines[line_no]);
        ++line_no;
    }

    fclose(file);

    for (int i = 0; i < line_no; ++i) {
        free(lines[i]);
    }
    free(lines);
}

Upvotes: 2

Majkl
Majkl

Reputation: 153

Ok, you have a couple of errors here:

  1. lines array is not declared

  2. Your allocation is wrong

  3. I don't understand this line, it is pointless to allocate something multiplying it by zero

    if( line_no == 0 ) {

    lines = (char**)malloc(sizeof(*lines) * line_no);

    }

  4. You shouldn't allocate array with just one element and constantly reallocate it. It is a bad practice, time-consuming, and can lead to some bigger problems later.

  5. I recommend you to check this Do I cast the result of malloc? for malloc casting.

You could write something like this:

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

void read_file(char const *name)
{
    int line_no = 0, arr_size = 10;
    int buffer_length = 1024;
    char buffer[buffer_length];
    char **lines;
    
    FILE* file;
    
    lines = malloc(sizeof(char*) * 10);
    
    file = fopen(name, "r");

    while(fgets(buffer, buffer_length, file)) {
        buffer[strlen(buffer)-1] = '\0';
        printf("---%s", buffer);
        ++line_no;
        
        if(line_no == arr_size)
        {
            arr_size += 10;
            
            lines = realloc(lines, sizeof(char*) * arr_size);
        }
        
        lines[line_no-1] = malloc(sizeof(buffer));
        lines[line_no-1] = buffer;
        printf("-------%s--------\n", lines[line_no-1]);
    }

    fclose(file);
}

PS, fgets() also takes the '\n' char at the end, in order to prevent this you can write the following line: buffer[strlen(buffer)-1] = '\0';

Upvotes: 0

Related Questions