CODEWITHSUNDEEP
csegmentation-faultmallocsizeof
Merve Sahin
Merve Sahin

Reputation: 1048

C segmentation fault

I'm trying to create a sub array with the following function : 

Track * subArray(Track * arr, int start, int end){
   int size = end - start;

   Track * t = malloc(sizeof(Track) * size);

      for(int i = 0; i < size && start <= end; i++){
          t[i] = arr[start++];

      } 
}

The size of the t pointer is always 8, even when I don't multiply it with size and I get a segmentation fault. I'm new to C so I don't know what is causing this exception.

Upvotes: 2

Views: 127

Answers (2)

csl
csl

Reputation: 11368

This is why C is hard. It's an off by one error: You need to allocate (end-start+1) items and use <= in the loop. Try rewriting to something like this:

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

typedef struct Track {
  char* color;
} Track;

Track * subArray(Track* arr, int start, int end){
  assert(end > start);

  const int size = 1 + end - start;
  printf("Allocating %d items\n", size);    
  Track* t = malloc(sizeof(Track)*size);

  for(int i=start; i <= end; ++i) {
    printf("at %d fetching %d\n", i-start, i);
    t[i-start] = arr[i];
  }

  return t;
}

int main() {
  Track *track = malloc(sizeof(Track) * 7);
  track[0].color = "red";
  track[1].color = "orange";
  track[2].color = "yellow";
  track[3].color = "blue";
  track[4].color = "indigo";
  track[5].color = "green";
  track[6].color = "violet";

  Track *sub = subArray(track, 3, 5);
  printf("%s\n", sub[0].color);
  printf("%s\n", sub[1].color);
  printf("%s\n", sub[2].color);
}

Compiling and running:

$ cc -g -W -Wall a.c && ./a.out
Allocating 3 items
at 0 fetching 3
at 1 fetching 4
at 2 fetching 5
blue
indigo
green

Note that I'm copying the value of char* pointers here. That may lead to additional confusing stuff, just in case you think about copying my code (I just drafted something that works to illustrate the problem).

Update

You're using inclusive indices. In C, however, it's quite common to specify a start index and a length. A lot of standard library functions do this, and this is what you'll most likely see in production code. One reason may be that it's easier to reason with. In your case, the code would be

Track* subArray(Track* arr, const size_t start, const size_t length) {
  Track* t = malloc(sizeof(Track) * length);

  for (size_t i = 0; i < length; ++i)
    t[i] = arr[i + start];

  return t;
}

and the corresponding call would be

Track *sub = subArray(track, 3, 3);

In my eyes, this not only looks better; it's simpler and easier to understand.

Another thing that's common is to copy pointers instead of the whole structs. This will depend on how your code and data structures are organized. In that case, it's quite common to use a sentry value at the end of an array of pointers to mark its end: This will typically be a NULL pointer.

Keep practising and keep reading other people's code, and you'll soon discover C idioms and programming styles that will make your life much easier!

Upvotes: 4

TowerFan
TowerFan

Reputation: 210

I think your error is that you're using <= in your tests when they should be <. This will prevent you from running off the end of your arrays.

Upvotes: 2

Related Questions