Skip to content

ext/sockets: Merge setting mcast option code for IPv4 and 6#18724

Closed
Girgias wants to merge 2 commits intophp:masterfrom
Girgias:socket-ipv6-4-merge
Closed

ext/sockets: Merge setting mcast option code for IPv4 and 6#18724
Girgias wants to merge 2 commits intophp:masterfrom
Girgias:socket-ipv6-4-merge

Conversation

@Girgias
Copy link
Copy Markdown
Member

@Girgias Girgias commented May 31, 2025

The handling for IPv4 and 6 seems similar enough that having two different functions doesn't seem to be worth it.

@devnexen
Copy link
Copy Markdown
Member

I understand the sentiment, hopefully these changes won t bring more issues than it tries to solve. Going to check on my mac what s wrong.

return php_do_mcast_opt(php_sock, level, optname, arg4);

case IP_MULTICAST_IF:
case IPV6_MULTICAST_IF:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah ... I m afraid you just can t do this that way. on mac those two has same values.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah and also Windows... Probably doable with some #if preprocessor directives tho.

@devnexen
Copy link
Copy Markdown
Member

do not see much of an option has having two still separated ipv4/ipv6 switches but within the same call (e.g. then passing ipv6 flag ?), not sure how much it s worth it though.

@Girgias
Copy link
Copy Markdown
Member Author

Girgias commented Jun 1, 2025

@devnexen do you know if IP_MULTICAST_TTL accepts a value of -1? I can't seem to find much info about it

@devnexen
Copy link
Copy Markdown
Member

devnexen commented Jun 1, 2025

@devnexen do you know if IP_MULTICAST_TTL accepts a value of -1? I can't seem to find much info about it

Normally you should get an EINVAL errno error.

@Girgias
Copy link
Copy Markdown
Member Author

Girgias commented Apr 6, 2026

Not sure if I want to remember what's going on in this PR if on top it doesn't seem to be worth it.

@Girgias Girgias closed this Apr 6, 2026
@Girgias Girgias deleted the socket-ipv6-4-merge branch April 6, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants