Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annihilate codepage & resolve startup segfault #238

Merged
merged 2 commits into from Sep 30, 2018

Conversation

@Headline
Copy link
Member

Headline commented Sep 28, 2018

I think this might have been the same case @KyleSanderson ran into. Not sure of it's cause, or why it has recently become a problem, but this should handle it just fine.

@@ -1088,14 +1088,16 @@ static void setconfig(char *root)

/* same for the codepage root */
# if !defined NO_CODEPAGE
*ptr='\0';
if(ptr)

This comment has been minimized.

Copy link
@dvander

dvander Sep 28, 2018

Member

I don't see a way ptr can be NULL here, as it's NULL checked above:

if ((ptr=strrchr(path,DIRSEP_CHAR))!=NULL || (ptr=strchr(path,':'))!=NULL) {

This comment has been minimized.

Copy link
@Headline

Headline Sep 28, 2018

Author Member

i think it’s reassigned and catches the plague here

if ((ptr=strrchr(path,DIRSEP_CHAR))!=NULL) {

This comment has been minimized.

Copy link
@Headline

Headline Sep 29, 2018

Author Member

for reference, here's my trace from the segfault
screen shot 2018-09-28 at 7 16 52 pm

This comment has been minimized.

Copy link
@dvander

dvander Sep 29, 2018

Member

Okay, thanks for pointing that out. After reading this crazy code I think I see what it's doing. It first tries to truncate "/path/to/spcomp" to "/path/to/" and then concatenate "include", to get "/path/to/include". If that path does not exist, it truncates to "/path/to", truncates again to "/path/", then concats to "/path/include".

At the end of all this, it expects ptr to be the "root" of where it found "include". This path is then used twice:

  1. Once for setting the codepage. We don't support codepages, they're a relic of a bygone era and we should mandate UTF-8 instead.
  2. Again for setting sc_rootpath, which is never used anywhere (I'm guessing we deleted whatever used it).

So my recommendation is that we just delete all the code after insert_path(path) in this function (line 1087 in my tree), and then delete sc_rootpath as well.

This comment has been minimized.

Copy link
@dvander

dvander Sep 29, 2018

Member

If anyone feels particularly bored/generous they could delete all the codepage stuff :)

@Headline Headline changed the title Fix random segfaults on some machines Annihilate codepage & resolve startup segfault Sep 30, 2018
@dvander

This comment has been minimized.

Copy link
Member

dvander commented Sep 30, 2018

Thanks!

@dvander dvander merged commit 7ee59e9 into master Sep 30, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dvander dvander deleted the segfault branch Sep 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.