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

Update ipv4.c #250

Closed
wants to merge 8 commits into from
Closed

Update ipv4.c #250

wants to merge 8 commits into from

Conversation

LightAutumnMelancholy
Copy link

This should fix a ton of the issues that come up in #154

Please look over my work, I'm new in C programming, I've been a shell and python and perl guy most of my career.

This should fix a ton of the issues that come up in #154
Forgot to add type for variable, and removed whitespace.
notes were too long.
line 141 was still to long/did not comply with coding style
Apologies for the long commit chain here, I'm just trying to get the style right so the build doesn't fail on style.
@LightAutumnMelancholy
Copy link
Author

LightAutumnMelancholy commented Feb 6, 2018

I took out some obvious white space, but astyle is still catching me somewhere.

Looks like whitespace was my culprit. That's what I get for trying to do this in a browser.
@adrienverge
Copy link
Owner

GitHub is showing the problem to you ;-)
problem

By the way you can run linting scripts locally to identify the problem.

@DimitriPapadopoulos
Copy link
Collaborator

Which OS does this patch target exactly?

@LightAutumnMelancholy
Copy link
Author

@adrienverge Thanks for that, the build passes now.

So I saw your comments about just calling netstat, I assumed that it was called with system() at some point down the line here.

@DimitriPapadopoulos non-macintosh unix/linux/bsd systems.

@DimitriPapadopoulos
Copy link
Collaborator

This part of the code is not executed on non-macintosh unix/linux/bsd systems - hence there isn't anything to fix here.

The "Input/output errors" are unrelated to netstat.

@LightAutumnMelancholy
Copy link
Author

LightAutumnMelancholy commented Feb 7, 2018

#ifndef __APPLE__
	int fd;
	// Cannot stat, mmap not lseek this special /proc file
	fd = open("/proc/net/route", O_RDONLY);
	if (fd == -1)
		return ERR_IPV4_SEE_ERRNO;

	size = read(fd, buffer, sizeof(buffer) - 1);
	if (size == -1) {
		close(fd);
		return ERR_IPV4_SEE_ERRNO;
	}
	close(fd);
#else
	FILE *fp;
	FILE *wnst;
	int len = sizeof(buffer) - 1;
	char *saveptr3 = NULL;

	// Open the command for reading
	wnst = popen("which netstat");
	fp = popen("wnst -f inet -rn", "r");
	if (fp == NULL)
return ERR_IPV4_SEE_ERRNO;

Please excuse me if I have this incorrect, but if the __APPLE__ token is set, this previous block of code executes, else the applicable block is executed?

Do I have this incorrect, can you help me understand if I have this incorrect?

@DimitriPapadopoulos
Copy link
Collaborator

DimitriPapadopoulos commented Feb 7, 2018

These mutliple ifdef/ifndef occurrences are indeed confusing (notice the missing/extra n):

  • ifdef __APPLE__if on Apple
  • ifndef __APPLE__if not on Apple

Ideally the code should be rewritten using only ifdef __APPLE__ for the sake of clarity.

@LightAutumnMelancholy
Copy link
Author

@DimitriPapadopoulos Thank you for helping me understand. I appreciate your time.

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.

3 participants