Using `strncpy` to copy overlapping strings.

[LeifAndersen]

Your STRNCPY macro (which just seems to call strncpy and then manually set the last byte to null), seems to frequently copy overlapping strings.

For example, in src/system/i18n.c, line 78, you have:

        if (strstr(language, ".") != NULL)
	{
		lang = strtok(language, ".");

                STRNCPY(language, lang, MAX_LINE_LENGTH);
	}

The use of strtok and strncpy seem to imply that the lang and language buffers overlap. Which, per the man page (and c spec) is undefined, and frequently causes segfaults:

"The strings may not overlap, and the destination string dest must be large enough to receive the copy." -- http://man7.org/linux/man-pages/man3/strcpy.3.html

If you want to copy to buffers that overlap, you either need to do it manually, or use functions like memmove (note that memcpy has the same problem).

As is, this currently causes builds to call SIGABRT on os x (10.11, using clang).

I built the game with:

$ make -f makefile.mac -j8
$ ./blobwarsAttrition
[1]    81299 abort      ./blobwarsAttrition

[stephenjsweeney]

Oh, that Mac makefile shouldn't be there. I was experimenting with the build but always faced the same issue of the segfault on start. I figured I'd done something wrong with it and meant to remove it.

Is the STRNCPY you've pointed out the source of this abort? The i18n.c file has been used in a couple of other projects and hasn't crashed there (valgrind doesn't report an issue either):

https://github.com/stephenjsweeney/tbftss/blob/master/src/system/i18n.c

https://github.com/riksweeney/edgar/blob/master/src/i18n.c

Maybe scrap the make file and start over, in case it's actually the source of the issue.

[LeifAndersen]

Possibly. I know when I change your STRNCPY macro to use memmove the game starts up, although only when I run it in lldb. (Which makes it difficult to debug. :( ).

But ya, here is the stack trace lldb gives me:

(lldb) bt
* thread #1: tid = 0x2209, 0x00007fff8d29df06 libsystem_kernel.dylib`__pthread_kill + 10, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
  * frame #0: 0x00007fff8d29df06 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff884e44ec libsystem_pthread.dylib`pthread_kill + 90
    frame #2: 0x00007fff880fc6e7 libsystem_c.dylib`abort + 129
    frame #3: 0x00007fff880fc85e libsystem_c.dylib`abort_report_np + 181
    frame #4: 0x00007fff88122a14 libsystem_c.dylib`__chk_fail + 48
    frame #5: 0x00007fff88122a24 libsystem_c.dylib`__chk_fail_overlap + 16
    frame #6: 0x00007fff88122a55 libsystem_c.dylib`__chk_overlap + 49
    frame #7: 0x00007fff88122ba6 libsystem_c.dylib`__strncpy_chk + 79
    frame #8: 0x0000000100018016 blobwarsAttrition`setLanguage + 342
    frame #9: 0x0000000100018f60 blobwarsAttrition`init18N + 224
    frame #10: 0x0000000100022faf blobwarsAttrition`main + 111
    frame #11: 0x00007fff83cb85ad libdyld.dylib`start + 1

Which corresponds to that block of code. I'll investigate further.

[stephenjsweeney]

Please try the latest code in the develop branch (includes some other bug fixes). This should get past the i18n.c string smashing. There might be others, but hopefully not.

[LeifAndersen]

Cool, it seems to start up for me now. :D

[LeifAndersen]

(although I should mention that full screen is not yet working.)

[stephenjsweeney]

Fullscreen requires you to exit and restart the game (unless you've done that and it still doesn't work?)

[LeifAndersen]

Ah, okay. Well in that case it works.

Thanks. :)