0

For some reason after coming back from years of not programming in C I cannot make this work:

(This compiles with no complain but it's causing a crash, when I remove the strcat line the executable runs fine)

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

int main(int argc, char **argv){
    char clibdir[50] = "C:\\Users\\______000\\Desktop\\nodeC\\libraries\\c";
    char varsfile[20] = "\\variables.xml";
    printf("%s\n", clibdir);  //ok
    printf("%s\n", varsfile); //ok
    char *varspath = strcat(clibdir, varsfile);  //this is provoking a crash
    printf("%s\n", clibdir);  //prints right before crash
    printf("%s\n", varspath); //prints right before crash
    return 0;
}

This prints just perfectly but it's crashing my exe. This is my command line (I'm using cl.exe from Visual Studio 2010):

"%vcbin%\vcvars32.bat" && "%vcbin%\cl.exe" /nologo /EHsc main.cpp /link /subsystem:console
shuji
  • 7,369
  • 7
  • 34
  • 49
  • It's not causing crash for me. – Iharob Al Asimi Jan 10 '15 at 01:58
  • [Works as indented](http://ideone.com/6YPZY6). – Kerrek SB Jan 10 '15 at 01:58
  • Could you run it in `gdb` and see where it crashes? – lared Jan 10 '15 at 02:01
  • Could you explain what you mean by "crash", and how you know that it is crashing? – Scott Hunter Jan 10 '15 at 02:05
  • Well, it appears a window with the text: main.exe stopped working. – shuji Jan 10 '15 at 02:06
  • I'd love to help, but it's compiling and running without any problems on my Linux machine. – chenshuiluke Jan 10 '15 at 02:07
  • @Sky, me too. And without compiling it, it looks perfect, it shouldn't crash, although it's a bit ugly. – Iharob Al Asimi Jan 10 '15 at 02:08
  • I think it's probably my compiler version or something, I dont know if there is some standard on how to run strcat. – shuji Jan 10 '15 at 02:09
  • @shuji Out of curiousity, which compiler and version do you have? – chenshuiluke Jan 10 '15 at 02:11
  • cl.exe from Visual Studio 2010, but the IDE is a little too heavy for my PC so i just use the command line. – shuji Jan 10 '15 at 02:12
  • This is not reproducible, try a different compiler, you can downlod `gcc` for windows. Or much better, you can use Linux. – Iharob Al Asimi Jan 10 '15 at 02:14
  • Not really, i develop for windows. And many libraries I use depend on the WinApi but I am not really used to C. – shuji Jan 10 '15 at 02:15
  • @shuji well your program has no error, and the crash must be caused by something else, AFAIK `gcc` for windows can be used with WinApi. – Iharob Al Asimi Jan 10 '15 at 02:19
  • all I can think of is if you had a typo and one of those `\n` were `%n` by accident that could cause a crash, – Jasen Jan 10 '15 at 02:21
  • Nope, they are all \n double checked :/ – shuji Jan 10 '15 at 02:22
  • You've avoided most of the usual mistakes; your printed lines end with a newline, you're returning a value to the invoking environment, etc. The only mild issue is that neither `argc` nor `argv` is used so maybe `int main(void)` would be better, but that's the limit of the damage. You say that the final two print statements output values, but the program crashes shortly after that? It's a little weird, to be polite. There's no obvious reason for a buffer overflow; the destination string is long enough. Have you rebooted your system recently? (Yes, it's not often necessary, but sometimes...) – Jonathan Leffler Jan 10 '15 at 02:23
  • A thought: is the compiler actually calling `strcat()`, or is it messing around with `strcat_s()` and automatically translating from one to the other behind the scenes? I don't rate this as a probable problem, but just looking for anything. Have you tried running it in the debugger? Are you compiling with warnings enabled? Are you getting any compilation warnings? (You said 'compiles clean', but was that with warnings enabled or not?) – Jonathan Leffler Jan 10 '15 at 02:24
  • 4
    `strcat()` would write to `clibdir` which is initialized with a string literal. Depending on compiler options `clibdir` could be translated into a pointer to protected data section. Writing at that location would result in undefined behavior. – SleuthEye Jan 10 '15 at 02:27
  • 4
    @SleuthEye No, it was declared `char[50]`, it'd go in a writable location, – Jasen Jan 10 '15 at 04:43
  • 3
    Yes, the clibdir variable was bigger than 50 characters, that caused the error. It should be fine with a 100 size. – shuji Jan 10 '15 at 04:47
  • yeah, that would do it, – Jasen Jan 10 '15 at 04:48
  • 2
    *How* do you expect us to debug your problems when you don't show us the code that actually crashes? The original version of the question showed `char clibdir[50] = "\\libraries\\c";` which is so short there's no problem. The revised version still has a variable that's only 50 characters long, but it is initialized with 45 characters, so adding another 14 or so to it overflows the buffer and, given the crash report, manages to trash the stack — that's called a [Stack Overflow](http://stackoverflow.com/). It's now trivial to see — but only because you show us the code that is actually crashing! – Jonathan Leffler Jan 10 '15 at 05:35
  • I'm really sorry man, I really thought the string was less than 50 characters but apparently the size increase very quickly. – shuji Jan 10 '15 at 16:13
  • possible duplicate of [strcat segmentation fault](http://stackoverflow.com/questions/4309577/strcat-segmentation-fault) – shuji Jan 10 '15 at 16:46

1 Answers1

3

Your code is crashing because you've not allocated enough space in clibdir to hold the initial string and the appended string, so you have a buffer overflow. The trouble is, you've trashed the return stack from your main() function, so the program goes haywire and crashes when you return from the main() program. You'd probably find that if you replaced the return 0; with exit(0);, your program no longer crashes. That's coincidental — not a recommended fix.

The moral of the story is "make sure there's enough space for the strings you append"!

The sane fix is to increase the size of clibdir from 50 to at least 60.

…And…when you ask a question, make sure that the code you show in the question actually crashes the same as the code you are running on your machine. The original version of the question had:

char clibdir[50] = "\\libraries\\c";

instead of:

char clibdir[50] = "C:\\Users\\______000\\Desktop\\nodeC\\libraries\\c";

and no-one could understand why the code was crashing — because, indeed, the original code should not crash.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278