C Program gives me segmentation fault 11

Question

So I believe that the problem lies in getting the reference number in the ref array as a character string and converting it to an integer to be converted over to the integer array results that is copied to the page array. Since i'm getting a segmentation fault 11 i'm going over the bounds of an array. Just not sure how to fix this.

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


int main()
{

  char ref[30];// array that holds reference string
  int frame_size;// maximum number of frames is 8
  // a frame holds a number thats in the reference stream.  
  int optimal_fault =0;
  int lru_fault =0;// least recently used faults
  int mfu_fault =0;// most frequently used faults
  int lfu_fault =0;// least frequently used faults    
  int pages = 0;//counts how many times you've looped
  //int page=0;//this will be the pages array 
  printf(" Please enter reference string: ");


  fgets(ref, 30, stdin);
  int num;

  //printf("reference: %c", &ref);
  printf(" Please enter the number of frames(no more than 8 allowed)\n");

  scanf("%d",&frame_size);

  int len = strlen(ref);
  int results[len];
  int page[len];
  int k=0;


  printf("len: %d:",len);
  for(int i=0; ref[i]!= '\0'; i++)
  { 
    if(isdigit(ref[i]))
    {
      num = sscanf(&ref[i], "%d", &results[i]);
      printf("results: %d\n", results[i]);
      page[k] = results[i];
      printf("page: %d\n", page[k]);
      k++;
      i++;
    }

  }
  return 0;
 }

Answer 1

I'm happy to work through the problems your program has, but you need to have an open mind and be willing to try fixing the issues I raise. I'll identify the errors. You make the obvious changes, and test it. I'm almost certain your error will disappear in the process. If they don't, post an update to the question with the changes you've made and ping me, and we'll work from there...

---

First, let us consider how this will execute when provided a string like "0".

It is expected that i would be incremented once for this string, because there's only one character in it, hence the loop looks like this: for(int i=0; ref[i]!= '\0'; i++)... There's no reason to believe this code will increment i more than once at this point, right?

However, your code increments again here:

if(isdigit(ref[i]))
{
  /* SNIP */
  i++;    // <--- ERROR HERE!
}

Your code skips the string-terminating '\0' byte because it increments too many times. It jumps off the end of the array, invoking undefined behaviour. It's realistic to expect that this might cause a segfault, however because it's UB there are no requirements.

---

isdigit expects that its arguments are unsigned char or EOF; any negative value that aren't EOF might cause assertion errors. Perhaps you meant to write: if(isdigit((unsigned char)ref[i]))

---

int len = strlen(ref);
int results[len];
int page[len];

For a start, strlen returns a size_t. The conversion from unsigned to signed integer types results in implementation-defined behaviour if the source value lies outside of the range of the destination type. That implementation-defined behaviour might (theoretically) include a trap representation, which could raise traps (i.e. segfaults).

Furthermore, have you considered that when len is 0 (e.g. you have an empty string) you're declaring 0-sized arrays? According to the C standard this is undefined behaviour.

... each time it is evaluated it shall have a value greater than zero.

Perhaps you meant to write something like this:

size_t len = strlen(ref);
if (len == 0) {
    puts("ref is too small! This field must be at least one byte...");
    return 0;
}
int results[len];
int page[len];
printf("len: %zu:",len); // NOTE: %zu causes printf to print a size_t; don't use %d for that.

Consider using size_t for other variables that denote sizes, such as i and k, as int may wrap around to negative values (which is a consequence of undefined behaviour, so technically it may crash or worse, instead). You wouldn't want to results[i] where i is negative, would you? (not that ref could have that many bytes, in this code)

---

int main() is not considered a valid entry point in standard C. You should be using int main(void), which specifies that the main entry point takes no arguments, unlike what you've used which specifies that the main entry point takes an unspecified number of arguments of unspecified type. This error occasionally causes segfaults and can be demonstrated by compiling these two programs:

int main() {
    main(42, "hello", -1.0); // WHAT?! main() can accept three arguments?!
}

 

int main(void) {
    main(42, "hello", -1.0); // Note the compiler error...
}

Again, this is UB so there are no requirements... Unfortunately I couldn't find any links in the ten minutes I spent searching, however I'm sure I've seen an instance of this causing segfaults at program termination.

---

You should (almost) always check the return value of scanf. scanf("%d",&frame_size); provides no guarantee of success, and thus no guarantee that frame_size will contain a sane value. Having said that, frame_size doesn't seem to be used in any logic here. If you need to read and discard an int value from stdin, you can do so using scanf("%*d");. That is the only situation where you (most likely) don't need to check the return value.

This goes for sscanf, too! I noticed you're saving the return value of sscanf here: num = sscanf(&ref[i], "%d", &results[i]);... however, you're not doing anything with it! As a result, you can't guarantee that results[i] contains a sane value. Such a shame...

num = sscanf(&ref[i], "%d", &results[i]);
if (num != 1) {
    continue;
}