-3

I have created a simple program to generate and store all combinations of numbers (up to 8 i this example) in binary in an array. However when i try and run the script the error message Segmentation Fault:11 appears. I can't figure out what is wrong..

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

int main()
{
      int no,i=0,j;
      int *d;
      d = (int *)malloc(sizeof(int)*50);
      for(no=0;no<8;no+=1)
      {
      do
      {
           d[i]=no%2;
           i++;
           no=no/2;
      }while(no>0);
      }
      for(j=i-1;j>=0;j--)
      {
            printf("%d",d[j]);
      }

 }
Mat
  • 202,337
  • 40
  • 393
  • 406
james
  • 11
  • what do you think the `do..while` loop is doing inside `for` loop with `no`? – Sourav Ghosh Jan 31 '15 at 15:08
  • 1
    also, please do not cast return value of `malloc()` and always check for the success of `malloc()`. – Sourav Ghosh Jan 31 '15 at 15:09
  • 1
    Did you step through the code in a debugger? Please read http://ericlippert.com/2014/03/05/how-to-debug-small-programs/\ – Eric Lippert Jan 31 '15 at 15:14
  • @EricLippert - I doubt it, since the problem would have been obvious. – Martin James Jan 31 '15 at 15:16
  • 4
    @MartinJames: Indeed it would. Many novice developers seem to have this idea that you find bugs by thinking about it really hard. I say: you caused the problem with a computer, so solve the problem with a computer. Use a debugger. Write assertions. Use compiler warnings. – Eric Lippert Jan 31 '15 at 15:18
  • @james: Now that you know where the bug is, don't just fix it. Ask yourself "how could I have avoided this bug by better program design?" and "how could I have found this bug automatically?" Learn from the mistake. For instance, had you made the inner loop a method of its own, you would not have written the bug in the first place. I internalized this rule the first time I made your mistake, and now I almost never write error-prone nested loops. A loop is non-trivial work, and non-trivial work goes into a method that does precisely one thing well. – Eric Lippert Jan 31 '15 at 15:21
  • ^^ what @EricLippert says. – Martin James Jan 31 '15 at 16:08

1 Answers1

0

The problem here is, with the do..while loop inside for loop

do
      {
           d[i]=no%2;
           i++;
           no=no/2;
      }while(no>0);

in place is ending up changing the value of no, thus making the for loop run apparently forever, thus writing past the allocated memory for d.

Solution: use a new variable, say int val to use with do..while loop, and set its value equal to no in the first statement in for loop.

Side note:

  1. Please do not cast the return value of malloc() and family.
  2. Always check for the success of malloc() [or, for that matter, any library function]
Community
  • 1
  • 1
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
  • Though you're on the right track here, the real bug in my opinion is that badly-named-variable `I` is never reset to zero. It just gets bigger and bigger. That the program also has an infinite loop is not the problem that is causing the segmentation fault. – Eric Lippert Jan 31 '15 at 15:23
  • @EricLippert Sir, I did not get the _reset to zero_ part. Can you please elaborate a bit? – Sourav Ghosh Jan 31 '15 at 15:29
  • Ah, I think this might have been my misunderstanding. I thought the intention of the program was to produce the array {0}, then the array {1}, then {1, 0}, then the array {1, 1}, and so on. But the program really is intended for some bizarre reason to produce {0, 1, 1, 0, 1, 1, ... }. So `i` is intended to increase every time through the inner loop, not to be reset to the beginning of the array on each number. This is a very strange little program. So: ultimately it is the fact that `i` gets too big that causes the fault, but it gets too big because of the infinite loop you identified. – Eric Lippert Jan 31 '15 at 15:51
  • @EricLippert Yes sir, once we get rid of the infinite loop, related problem should be solved. Nevertheless, thanks for making the point. :-) – Sourav Ghosh Jan 31 '15 at 16:03
  • Thank you for your help Sourav and Eric. The reason I want to store a string of binary combinations is because I later want to extract them from the array and assign blocks of text to the values of 1 and 0 in correspondence with their position. This is because I am writing a large program that involves combinations of text and therefore I am searching for a way to automatically print out all the combinations. I have removed the infinite loop but the program still shows the Segmentation Fault: 11 error :( – james Jan 31 '15 at 17:56