0

For some homework I have to write a calculator in C. I wanted to input some string with scanf and then access it. But when I access the first element I get a segmentation error.

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

int main(){
char input1[30];

scanf("%s",input1);
printf("%s",input1);

char current = input1[0];
int counter = 0;
while(current != '\0'){

if(isdigit(current) || current == '+' || current == '-' || current == '*' || current == '/'){
 counter++;
 current = input1[counter];
}else{
 printf("invalid input\n");
 exit(1);
}

}

return 0;
} 

The printf in line 3 returns the string, but accessing it in line 4 returns a segmentation error (tested in gdb). Why?

Kunait
  • 31
  • 4
  • 5
    How long was the string you entered? Also, you never initialize counter. You need to do that. – Avi Berger Nov 04 '22 at 18:17
  • 3
    What value does `counter` start out as? – Steve Summit Nov 04 '22 at 18:19
  • 1
    try `scanf("%29s",input1);` – Jean-François Fabre Nov 04 '22 at 18:20
  • Questions of a debugging nature should include the shortest amount of code to reproduce the symptoms, but this has gone too far, to the point where the symptoms we see are different to what you described. How are you getting a segfault from that which cannot compile? For all we know, you may not realise a `main` entry point is required for all of that code, so your binary may be a staple version of code that did compile (but had logic errors). If you're gonna assert that it compiles in order to produce the symptoms, then please make sure what you give us also compiles to produce the symptoms. – autistic Nov 04 '22 at 19:59
  • See also, ["How to create a Minimal, Reproducible Example"](https://stackoverflow.com/help/minimal-reproducible-example) – autistic Nov 04 '22 at 20:00
  • @autistic I'm sorry, this is my first post here. I just left out the main method, there is nothing else to the code yet, just including stdio and the main method. But you're right, I am sorry – Kunait Nov 04 '22 at 23:48
  • @AviBerger Edited and initalized counter, my input was just one char as a test, the segmentation fault occurs even when accessing the first char so counter was never really used – Kunait Nov 04 '22 at 23:53
  • @SteveSummit Please read my comment – Kunait Nov 04 '22 at 23:54
  • `current = input1[counter];` with an uninitialized counter is likely to be an out of bounds access and might well produce a segfault. – Avi Berger Nov 05 '22 at 00:23
  • @AviBerger the problem is the segmentation fault occurs before that, it occurs at char current = input1[0]; – Kunait Nov 05 '22 at 06:29
  • What happens if you add `#include ` (for `isdigit`) and `#include ` (for `exit`)? – Adrian Mole Nov 05 '22 at 06:50
  • @AdrianMole in my code I had it included, forgot to copy it :/ Gonna edit again – Kunait Nov 05 '22 at 13:55
  • Can you be clearer about which line is "line 3" and which line is "line 4". There have been edits to the code which invalidated old line numbers. In general, we prefer to use comments to mark line of code under discussion instead of making people count lines. – Raymond Chen Nov 05 '22 at 14:02
  • I can't reproduce this. If I give the program an input consisting of a single character and then newline (e.g. by typing `x` [Enter]) then it prints `invalid input` and doesn't segfault. The only way I can see for this to crash is if you input a string longer than 29 characters (and that, of course, is a buffer overflow security flaw, so if you write `scanf("%s")` in real-life code, expect to be fired). – Nate Eldredge Nov 05 '22 at 14:04
  • Other than the uninitialized `counter` (which is now fixed), I can't see anything wrong with this code. I tried the fixed code on my computer, and got no errors. So I can't reproduce this, either. Whatever problem you're having, it's in something we can't see, so I'm afraid it's likely we're not going to be able to help you fix it, sorry. – Steve Summit Nov 05 '22 at 19:55
  • I suggest reading the entire question after any edits, just to make sure all of the details still make sense. I know what you mean by lines 3 & 4, but they're not actually lines 3 & 4, right? Also, you can expect an answer from me soon, but in typical form I'ma cite the FAQs and other resources you probably ought to be reading prior to coming here to ask this question... so if I may ask, which book/s are you reading to study C? – autistic Nov 05 '22 at 20:29
  • @autistic I am taking an Uni course, but we are still pretty much at the beginning – Kunait Nov 06 '22 at 02:23
  • @Kunait That's not an answer to my question. If you're taking a university course to study C, your course instructor has probably told you which book/s you're reading to study C. You're probably also expected to do exercises from that book, in your own time. Your book should guide you swimmingly through the distinctions between `scanf` with the `%s` directive, `fgets` and `fgetc`, past the pitfalls of `` functions, basically all of these things I explained in my answer. If it doesn't, it's a poor book; you need a new one. If it does, it'll be faster to **read it before asking**... – autistic Nov 06 '22 at 20:06
  • @autistic we are not working with a book right now – Kunait Nov 06 '22 at 22:03

1 Answers1

0

There are a few potential causes, some of which have been mentioned in the comments (I won't cover those). It's hard to say which one (or more) is the cause of your problem, so I guess it makes sense to iterate them. However, you may notice that I cite some resources in the process... The information is out there, yet you're not stumbling across it until it's too late. Something needs to change with how you research, because this is slowing your progress down.


On input/output dynamics, just a quick note

printf("%s",input1);

Unless we include a trailing newline, this output may be delayed (or "buffered"), which may have the effect of confusing you about the root of your issues. As an alternative to using a trailing newline (which I'd prefer, personally) you could explicitly force partial lines to be written by invoking fflush(stdout) immediately after each of the relevant output operations, or use setbuf to disable buffering entirely. I think this is unlikely to be your problem, but it may mask your problem, so it's important to realise, when using printf to debug, it might be best to include a trailing newline...


On main entry points

The first potential culprit I see is here:

int main()

I don't know why our education system is still pushing these broken lessons. My only guess is the professors learnt many years back using the nowadays irrelevant Turbo C and don't want to stay up-to-date with tech. We can further reduce this to a simple testcase to work out if this is your segfault, but like I said, it's hard to say whether this is actually your problem...

int main() {
    char input1[30];
    memset(input1, '\x90', sizeof input1);
    return 0; // this is redundant for `main` nowadays, btw
}

To explain what's going on here, I'll cite this page, which you probably ought to go and read (in its entirety) once you're done here:

A common misconception for C programmers, is to assume that a function prototyped as follows takes no arguments:

int foo();

In fact, this function is deemed to take an unknown number of arguments. Using the keyword void within the brackets is the correct way to tell the compiler that the function takes NO arguments.

Simply put, if the linker doesn't know/can't work out how many arguments are required for the entry point, there's probably gonna be some oddness to your callstack, and that's gonna occur at the beginning or end of your program.


On input errors, return values and uninitialised access

#include <assert.h>
#include <stdio.h>
#include <string.h>
int main(void) {
    char input1[30];
    memset(input1, '\x90', sizeof input1);
    scanf("%s",input1); // this is sus a.f.
    assert(memchr(input1, '\0', sizeof input1));
}

In my testcase, I actually wrote '\x90' to each byte in the array, to show that if the scanf call fails you may end up with an array that has no null terminator. If this is your problem, this assertion is likely to throw (as you can see from the ideone demo) when you run it, which indicates that your loop is likely accessing garbage beyond the bounds of input1. On this note I intended to demonstrate that we (mostly) cannot rely upon scanf and friends unless we also check their return values! There's a good chance your compiler is warning you about this one, so another lesson is uto pay close attention to warning messages, and strive to have none.


On argument expectations for standard library functions

For many standard library functions it may be possible to give input that is outside of the acceptable domain, and so causes instability. The most common form, which I also see in your program, exists in the form of possibly passing invalid values to <ctype.h> functions. In your case, you could change the declaration of current to be an unsigned char instead, but the usual idiom is to put the cast explicitly in the call (like isdigit((unsigned char) current)) so the rest of us can see you're not stuck in this common error, at least while you're learning C.


Please note at this point I'm thinking whichever resources you're using to learn aren't working, because you're stumbling into common traps... please try to find more reputable resources to learn from so you don't fall into more common traps and waste more time later on. If you're struggling, check out the C tag wiki...

autistic
  • 1
  • 3
  • 35
  • 80
  • @SteveSummit *"consider this: any interface specification must be adhered to by both sides unless renegotiated."*... It's interesting to see your own words at times, huh? *"In fact, the new revision of the ANSI/ISO C Standard ("C99") actively disallows the first of these..."* In fact, `int main()` was deprecated as of C89, and so anyone who writes compilers that focus on compliance and linting is likely to disagree with you... I'm not sure if you realise, I'm writing my own implementation, so I guess I'm a part of this *"renegotiation"* you spoke of... Right? – autistic Nov 06 '22 at 20:18
  • I'm sorry, but I really have no idea what point you're trying to make. As of today, I believe `int main() {}` is still perfectly acceptable. – Steve Summit Nov 06 '22 at 20:39
  • @SteveSummit between these words I quoted earlier and [c89/3.9.4](https://port70.net/~nsz/c/c89/c89-draft.html#3.9.4) lies some divorce from reality. Perhaps you think it's okay to make use of deprecated features, whilst holding the belief that such expectations need to be established from both sides. Perhaps you're not yet aware that the C89 rationalised this as undefined behaviour, by pointing out that it's a class of error that ["it is extremely difficult for the translator to detect errors (wrong number or type of arguments)"](https://port70.net/~nsz/c/c89/rationale/c5.html#3-5-4-3). – autistic Nov 06 '22 at 20:52
  • What I am aware of is that the C Standards (AFAIK all versions) make a very special exception for a function named `main`: it is explicitly allowed to be defined as accepting either zero arguments, or two. If an architecture would otherwise have problems with arguments being passed but not used, it will have to do something special to make sure that `main` doesn't fail if the run-time startup code passes `argc` and `argv`, but the program doesn't use them. – Steve Summit Nov 06 '22 at 21:16
  • Also, remember that a function *declaration* with empty parentheses is subtly but significantly different from a function *definition* with empty parentheses. – Steve Summit Nov 06 '22 at 21:18
  • @SteveSummit `int main() { return main("accepting zero arguments hmmm?"); }` – autistic Nov 06 '22 at 21:33
  • Nicely quoted and updated, but the problem with your setup is that `int main() {}` (unlike "ain't", or `void main() {}`) is *not* undefined. `int main() {}` is 100% equivalent to `int main(void) {}`, and is therefore perfectly well defined. – Steve Summit Nov 06 '22 at 21:59
  • *the problem with int main() is that it accepts any number of arguments of any type* — That's true of the declaration `int main();`. It is *not* true of the definition `int main() {}`. – Steve Summit Nov 06 '22 at 22:04
  • @autistic Thank you for your answer, the problem was with my counter variable, but I couldn't diagnose it correctly because I forgot the \n in the printf statement, so the buffer didn't flush. I thought the error must have occurred before the print but it came after it, the program just didn't have the time to flush the buffer before having the segmentation fault. – Kunait Nov 06 '22 at 22:05
  • @SteveSummit `int main() { return main("how is this 100% equivalent?"); }`... Are you ignoring evidence to the contrary? The implementation doesn't define a prototype, and neither does your K&R style "function definition". In some situations, such as when we halt compilation prior to linking, the implementation cannot easily know how many arguments are present. You can see this isn't equivalent, because when `int main(void)` is used instead you get the mandatory diagnostic. Please refrain from making these clearly exaggerated claims of "100% equivalent" that don't stand up to scrutiny. – autistic Nov 06 '22 at 22:15
  • We will have to agree to disagree, then. This comment thread is far too long already. Perhaps I'll post my own question and let the language lawyers weight in, or you may wish to, yourself. – Steve Summit Nov 06 '22 at 22:29