-1

Note: I am still pretty new to C.

So I have a pretty overcomplicated program that should output FizzBuzz:

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

int main() {
    char* theArray[] = {"", "", "Fizz", "", "Buzz"};
    int size = sizeof(theArray)/sizeof(theArray[0]);
    for (int i = 1; i < 100; i++) {
        char* output = "";
        for (int j = 0; j < size; j++) {
            if (i % (j+1) == 0) {
                strcat(output, theArray[j]);
            }
        }
        if (output) { 
            printf("%d", i); 
        } else { 
            printf("%s", output); 
        }
        printf("\n");
    }
    return 0;
}

Compiling this produces no error while running it doesn't produce the desired FizzBuzz but just this error:

Illegal instruction: 4

I tried compiling it with the compiler-flag -mmacosx-version-min=12.0 since that is the version I'm currently on and also -mmacosx-version-min=10.x, since it is suggested by this question, however, nothing helped.

gurkensaas
  • 793
  • 1
  • 5
  • 29
  • 5
    `strcpy(output, output)` writes to a constant string, which is a big no-no (it's Undefined Behaviour). `strcpy` does not allocate a new string - it simply copies the memory from one string to another. – nneonneo Sep 30 '21 at 17:54
  • @nneonneo What would I do instead? As stated above, I'm new to C so I don't know any language conventions. – gurkensaas Sep 30 '21 at 17:55
  • Is there a reason you're using `fmod` on integers? I can't see a way for `if (output)` to ever be false and I'm not sure why you're trying to copy strings to themselves or what you think that would do. – Retired Ninja Sep 30 '21 at 17:55
  • C strings don't work like they do in other languages - in particular, you have to allocate storage for strings manually (unless they're constant - in which case they are not writable). There's no meaningful way I could answer that in a comment, so instead I would suggest reading a suitable C textbook or programming guide. – nneonneo Sep 30 '21 at 17:57
  • @RetiredNinja No, not really... I'm sorry, I wrote this code a pretty long time ago and I have no idea why I made a decision or another. – gurkensaas Sep 30 '21 at 17:57
  • @TimRandall "...without having your exact development environment..." I included GCC in the tags, somewhat described the way I compile, and provided the entire code I have. What should I provide more? – gurkensaas Sep 30 '21 at 18:08
  • @gurkensaas I don't think that that's the side of the problem to be attacked. I think that you need to relax your restriction that people not comment on whatever errors they see in your code. – Tim Randall Sep 30 '21 at 18:09
  • You have doubtlessly re-written the return address for this function which would be stored on the stack higher up than the string *output*. Strcat() is doing this. When this function eventually returns to garbage -- thus the illegal opcode. So first, launch your program in a debugger and verify that; then study how strings work in C, and change your program correspondingly. – mevets Sep 30 '21 at 18:10
  • [Here's a list of books to help you get started](https://stackoverflow.com/questions/562303/the-definitive-c-book-guide-and-list). C is not a language that you can learn experimentally. You need to study the language (especially how strings work) before you can use it. – user3386109 Sep 30 '21 at 18:13
  • Additionally, from the [`strcpy` man page](https://man7.org/linux/man-pages/man3/strcpy.3.html) _The strings may not overlap,_ so this is UB even if `output` (and `theArray[j]`) weren't string literals with enough size. What are you trying to accomplish there anyway? Copying a string to itself is a functional no-op. – yano Sep 30 '21 at 18:16
  • @yano I was reading the [first source which I could find](https://www.tutorialspoint.com/c_standard_library/c_function_strcat.htm) and saw that they used `strcpy`, so I tried to replicate their approach. – gurkensaas Sep 30 '21 at 18:19
  • your link is to `strcat`, that's for _concatenating_ strings. `strcpy` is for _copying_ the source string to the destination. Even if `strcpy(output, output);` was legal, `output` would have exactly the same data before and after. Did you mean for that to be `strcat(output, output);`? – yano Sep 30 '21 at 18:24
  • @yano No, I thought it might fix it. As you can see, there is a `strcat` just below the two illegal `strcpy`-s. I will update the code to remove all errors that were pointed out. – gurkensaas Sep 30 '21 at 18:29
  • yes, now I see the `strcpy, strcpy, strcat` in your link. The differences there are 1) `src` and `dest` are both writable memory areas. 2) They're both uninitialized until `strcpy` is called, which writes "This is source" and "This is destination". After those calls, `src` and `dest` contain those strings. Once they contain those strings, calling `strcpy(src, src);` (for example) would be useless (even it was legal).. you'd simply be overwriting what's there with itself. – yano Sep 30 '21 at 18:34
  • @yano I have now removed the two `strcpy`s and improved the question by using the right modulo operator (`%` instead of `fmod()`), but the error stays the same. – gurkensaas Sep 30 '21 at 18:37
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/237677/discussion-between-yano-and-gurkensaas). – yano Sep 30 '21 at 18:38
  • depending on what standard of c you are compiling, you might need to declare your variables of `i` and `j` at the beginning of the function. – dmaelect Sep 30 '21 at 18:56
  • 1
    @dmaelect I fixed it just by changing the output from `char* output` to `char output[100]` and `if(output)` to `if(strlen(output) == 0)`. – gurkensaas Sep 30 '21 at 19:09
  • @gurkensaas cool! – dmaelect Sep 30 '21 at 19:12
  • Note you have to also initialize your `char output[100]` array to make it an empty string, at least its first character. So add `output[0] = '\0';` after you declare it. – Nate Eldredge Sep 30 '21 at 19:47

1 Answers1

1

C doesn't have a string class like most languages. And that string class which it does not have, is not char*.

Each char* needs to point at memory allocated somewhere, or you can't use it. If you set it to point at string literals, "these things", then you must be aware that they are read-only. And only of as big a size as necessary to contain the string written to them. They don't re-size either, so something like "" is a pretty useless thing to have.

This means that you have to study arrays, then pointers, then strings, in that order. You haven't, so your program is full of bugs where you write to pointers which don't point to sufficiently allocated space, or to read-only locations. Start by reading this:

Crash or "segmentation fault" when data is copied/scanned/read to an uninitialized pointer
How should character arrays be used as strings?
What is the difference between char s[] and char *s?

Lundin
  • 195,001
  • 40
  • 254
  • 396