0

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);
}
Danial
  • 542
  • 2
  • 9
  • 24

2 Answers2

2

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);
}
Jorengarenar
  • 2,705
  • 5
  • 23
  • 60
  • can you check my question again ?? – Danial Jul 12 '20 at 17:29
  • really appreciate it, thanks a lot.. just one more question ```for(int i=0;(lines+i);i++){ printf("%s\n", *(lines+i));}``` if write this to read all those lines, it prints all line and crashes.. can you tell me why ?? – Danial Jul 14 '20 at 10:21
  • Don't use unnecessarily pointer notation. And `lines` isn't string literal - it doesn't have any end marker. You cannot use `lines+i` as loop condition as it's meaningless. It's a memory you don't have access to. – Jorengarenar Jul 14 '20 at 12:38
  • `lines_no` is the size of `lines` - instead of `(lines+i`)` condition should be `i < lines_no`. – Jorengarenar Jul 14 '20 at 17:31
  • `printf("%s\n", lines[i])` is more readable than `printf("%s\n", *(lines+i))`. Or just `puts(lines[i])` – Jorengarenar Jul 14 '20 at 17:32
0

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';

Majkl
  • 153
  • 1
  • 7