-1

I am writing a code which reads a text file in to chunks of memory of max 1024 bytes. For this i am creating a linked list with 1016 bytes of data and a pointer to the previous node. My code executes perfectly and dynamically allocates and uses data, as well as linking back perfectly. The problem arrises when it has to create the fourth node. When i increase the malloc size manually (and set it to 1200 for example) it creates 48 nodes before crashing, suggesting the struct size increases. But when i print sizeof(*memory) or sizeof(struct Chunk) the size stays 1024 bytes.

I get the following error caused by the line where i use malloc:

malloc.c:2392: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed. Aborted (core dumped)

my code is as following:

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

int main(int argc, char **argv) {

      // declare variables
  int const CHUNK_SIZE = 1024;
  int chunk_index = 0;
  struct Chunk {
    struct Chunk *previous;
    int data[(CHUNK_SIZE-sizeof(struct Chunk*))/sizeof(int)];
  };
  struct Chunk* memory = (struct Chunk *)malloc(sizeof(struct Chunk));
  struct Chunk* temp;

    // check if the amount of arguments is as expected
  if (argc!=2) {
    printf("Usage: reverse <filename>\n");
    return -1;
  }

      // check if the file can be opened
  FILE *fp;
  fp = fopen(argv[1], "r");
  if (fp==0) {
    printf("Cannot open file!\n");
    return -1;
  }

      // start program
  do {
    memory->data[chunk_index] = fgetc(fp);
    chunk_index++;
    if ( chunk_index*sizeof(int) > CHUNK_SIZE-sizeof(struct Chunk*) ) {
      temp = (struct Chunk *)malloc(CHUNK_SIZE);
      temp->previous = memory;
      memory = temp;
    }
  }
  while(memory->data[(chunk_index-1)]!=EOF && chunk_index<CHUNK_SIZE-sizeof(char*));
}
  • Are you sure `malloc(CHUNK_SIZE);` you want to do this? – cs95 Sep 26 '17 at 21:26
  • 1
    "My code executes perfectly" is a bold statement! – yano Sep 26 '17 at 21:28
  • gcc sez `warning: a member of a structure or union cannot have a variably modified type`. Your code is invalid. – EOF Sep 26 '17 at 21:31
  • Why are you setting the size of `data[]` in such a complicated way? Why not just say `int data[DATA_SIZE]`, for some appropriate value of `DATA_SIZE`? – Steve Summit Sep 26 '17 at 21:33
  • Also, if you are storing characters, why is `data` an array of `int`? (Yes, if it were an array of `char`, you'd have a hard time reliably testing for EOF, but there are better ways around that problem than making the whole array an array of `int`.) – Steve Summit Sep 26 '17 at 21:35
  • Anyway, the error you're getting usually means that you wrote to more memory than you asked for, which is not surprising given the overcomplicated way you're computing how much to ask for. – Steve Summit Sep 26 '17 at 21:37
  • I recommend compiling with `address-sanitizer` or running under `valgrind`. The actual errors are quite simple and would be easy to spot if you didn't obfuscate your struct declaration. – EOF Sep 26 '17 at 21:38

1 Answers1

0

Code has trouble when allocating new memory as it does not reset chunk_index. Eventually code attempts to access outside allocated memory->data[].

  int chunk_index = 0;
  int ch;  //  add, last value read 

  do {
    ch = fgetc(fp);  // save ch
    memory->data[chunk_index] = fgetc(fp);
    chunk_index++;
    if ( chunk_index*sizeof(int) > CHUNK_SIZE-sizeof(struct Chunk*) ) {
      temp = (struct Chunk *)malloc(CHUNK_SIZE);
      temp->previous = memory;
      memory = temp;
      chunk_index = 0; // ** add **
    }
  }
  // ** replace **
  // while(memory->data[(chunk_index-1)]!=EOF && chunk_index<CHUNK_SIZE-sizeof(char*));
  while(ch !=EOF && chunk_index<CHUNK_SIZE-sizeof(char*));

I have doubt that chunk_index<CHUNK_SIZE-sizeof(char*) is correct. It is likely incorrect as the units do not match up. chunk_index indexes an array (example a +4 address change each time chunk_index increments yet CHUNK_SIZE-sizeof(char*) is measuring in bytes.) OP will need to review this. I'd expect while(ch !=EOF); to be sufficient.


Further, I would add a new block when needed. Presently code links a new block to be prepared for the next fgetc(fp), which may not happen. By adding a new block just before the fgetc(fp), code would not even need the prior memory = (struct Chunk *)malloc(sizeof(struct Chunk)); code and could use memory = NULL;


Tip: Instead of casting and allocating to a constant, drop the cast and allocate to the size of the referenced variable. Easier to code correctly, review and maintain.

// temp = (struct Chunk *)malloc(CHUNK_SIZE);
temp = malloc(sizeof *temp);
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256