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

Simplified version of IPv6 changes, this is investigational and a WIP #354

Closed
wants to merge 1 commit into from

Conversation

gcasa
Copy link
Member

@gcasa gcasa commented Nov 29, 2023

I need this on a branch at the moment to test with some functionality at keysight. Adding it here as a WIP, this is NOT ready for review.

@gcasa gcasa self-assigned this Nov 29, 2023
Copy link
Contributor

@rfm rfm left a comment

Choose a reason for hiding this comment

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

All the changes in GSSocketStream.m do is add explicit casts to say that variables are of the type that the compiler already knows they are.
That's not actually harmful because it is a no-op, but it's bad style to add unnecessary casts, and makes the code a little harder to read.

The change in win32/NSStream.m is basically already in master (so would probably conflict with the existing code). The difference is that, in master it's protected by checking for PF_INET6 being defined rather than AF_INET6, but I think on any system where one is defined both are defined.
If the code in master was wrong on this point (ie PF_INET6 was undefined) inet6 streams could not work at all on windows, but the automated regression tests are passing on windows systems so we know they do.

@gcasa
Copy link
Member Author

gcasa commented Nov 29, 2023

All the changes in GSSocketStream.m do is add explicit casts to say that variables are of the type that the compiler already knows they are. That's not actually harmful because it is a no-op, but it's bad style to add unnecessary casts, and makes the code a little harder to read.

The change in win32/NSStream.m is basically already in master (so would probably conflict with the existing code). The difference is that, in master it's protected by checking for PF_INET6 being defined rather than AF_INET6, but I think on any system where one is defined both are defined. If the code in master was wrong on this point (ie PF_INET6 was undefined) inet6 streams could not work at all on windows, but the automated regression tests are passing on windows systems so we know they do.

I already have a strong suspicion that the issue is with the application and not with GS code. I should know more over the next few days.

@gcasa
Copy link
Member Author

gcasa commented May 22, 2024

Summarily closing this issue as it is invalid and outdated.

@gcasa gcasa closed this May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants