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

Possible fix for tmpnam. #308

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

thyrgle
Copy link
Contributor

@thyrgle thyrgle commented Oct 19, 2015

According to here you should not use tmpnam. I am not familiar with Socket programming, so I am not sure how to test. I believe I have fixed it by replacing it with mkstemp, but I'm not completely sure. Nevertheless, this is an issue that should be addressed.

@stevedekorte
Copy link
Member

Thanks Chris. Do we know if this works on all platforms?

@thyrgle
Copy link
Contributor Author

thyrgle commented Oct 19, 2015

I am not certain.

@@ -157,7 +157,8 @@ void LocalNameServers_findIps(LocalNameServers *self)

void LocalNameServers_findIps(LocalNameServers *self)
{
char *path = tmpnam(NULL);
char *path = NULL;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's a minor whitespace issue here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny, the whitespace doesn't show up incorrectly on my computer. Probably some issue with my vim configuration.

@joedf
Copy link

joedf commented Oct 19, 2015

mkstemp() seems to be part of the GNU C Library.

@acook
Copy link

acook commented Oct 19, 2015

@stevedekorte It looks like it's supported at least on FreeBSD, Linux, OSX, and Solaris.

@stevedekorte
Copy link
Member

Can anyone test it on Windows?

@acook
Copy link

acook commented Oct 22, 2015

Looks like it's deprecated on Windows.

@stevedekorte
Copy link
Member

If someone would like to mod the patch to work on windows, I can accept it.

@ghost
Copy link

ghost commented Jan 22, 2016

Hope you don't mind me jumping in but:

  • the code with tmpnam is only used when __APPLE__ is defined.
  • The VS2015 reference for _mkstemp that @acook referenced is for _mktemp which is different.
  • As far as I can see mkstemp needs to be called with a template argument which is overwritten, not NULL (I can't test this at the moment)

So I think you just need a solution for OS X, probably using mktemp. Also popen might be an option.

@ghost
Copy link

ghost commented Jan 22, 2016

Quick thought on using popen - good because it avoids all the temporary file issues.

#include <stdio.h>

int main(void) {
  static char answer[1024];
  FILE* fp = popen("dig|grep SERVER:", "r");
  if (fp) {
    fread(answer, 1, 1024, fp);
    /* Do something with 'answer' ... */
    puts(answer);
    pclose(fp);
  } else {
    /* Error */
  }
}

Tested on Linux, not yet on OS X.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants