1

Apologizes I am very inexperienced with C. I have the following code: 

char * a[BUF_SIZE]; 

scanf("%d", numberOf);
do {
    a[i] = (char *)malloc(MAX_LINE_LEN + 1);
    scanf("%s", a[i]);
    ++i;
} while(i < numberOf);

The idea is simple, read two inputs from stdin using scanf, the first being a single int, followed by some array of strings. Scanf works independently in both cases e.g. scanf("%d", numberOf) will store a digit and scanf("%s", a[i]) will store a set of strings into the array. However in conjunction reading an integer first into numberOf causes a segfault when reading in a set of strings. My question is why? I know its generally bad practice to use scanf, but I fail to see how reading in multiple inputs from stdin can cause a segfault in the resulting code. Much Thanks!

user2808315
  • 117
  • 12
  • 3
    Decide on one language. – cadaniluk Sep 18 '15 at 18:16
  • 2
    1) You should check whether `numberOf > BUF_SIZE`. 2) Replace the do-while-loop with a for-loop. 3) On casting the return value of `malloc`: http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc 4) Call `free`. – cadaniluk Sep 18 '15 at 18:21
  • @cad I agree with 1), as for 2) could you explain why exactly there would be an advantage in converting the looping semantics, I can't seem to find any reason to support a change other than readability, for 3) I am compiling with g++ as a user commented in the post "Considering that C++ compilers give better warnings than C compilers, making your code C++ compilable is worth the downside IMHO". – user2808315 Sep 18 '15 at 18:27
  • 1
    If you use a C++ compiler then why did you remove the C++ tag, keeping the C tag? When using C++ you should use `new` and `delete` instead, if you didn't know. On your question regarding the loop: every normal C programmer will instantly associate the for-loop with an index and such, thus making it more readable. Anyway, the more practical fact is that a do-while-loop runs at least one time; however, what if `numberOf == 0`? – cadaniluk Sep 18 '15 at 18:31
  • @cad Awh, good point. As for the tags, it is a C language related question specifically. I'm not interested in a C++ language specific solution. Not specified in the post, so I can understand the confusion, my task has at least one input always, so I utilized a do-while simply because it's my first time exposed to the looping structure and I feel it's appropriate considering the loop should always run at least once. I appreciate the feedback. – user2808315 Sep 18 '15 at 18:38

2 Answers2

5

From the code, it looks like numberOf is an int. When using scanf, you want to pass it a pointer, so change scanf("%d", numberOf); to scanf("%d", &numberOf);

What scanf does is take the user input and put it into the memory address specified by the second parameter. When you provide an int as the second parameter, scanf tries to put user input into a memory address (specified by the int) that it may not own, causing the seg-fault.

R_Kapp
  • 2,818
  • 1
  • 18
  • 32
  • Do you think you could explain why scanf causes a segfault when not passed a pointer for educational purposes? Much appreciated. – user2808315 Sep 18 '15 at 18:32
  • 3
    @user2808315: The value contained in `numberOf` is most likely not a valid pointer value (i.e., not an address that your code may write to), which *may* cause a segfault. – John Bode Sep 18 '15 at 18:41
4

You are missing & operator in scanf("%d", numberOf); put it as &numberOf

vvs14
  • 720
  • 8
  • 19