Network: fix regression errors#2051
Conversation
WalkthroughThe changes update two components. In the wireless attributes function, direct array access is replaced with the null coalescing operator in Changes
Sequence Diagram(s)sequenceDiagram
participant Script as Netconfig Script
participant RCInet1 as "/etc/rc.d/rc.inet1"
participant Ini as "create_network_ini"
participant WG as "update_wireguard"
Script->>RCInet1: Execute "{ifname}_stop"
Script->>RCInet1: Execute "{ifname}_{cmd}" (start interface)
Script->>Ini: Execute "create_network_ini {ifname}"
alt dns not set
Script->>WG: Call update_wireguard({ifname})
end
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
emhttp/plugins/dynamix/include/Wireless.php (1)
103-105: Use of null coalescing operator enhances error handling.The addition of the null coalescing operator (
??) provides a fallback empty string value when the specified keys don't exist in the$wlan[$index]array. This prevents PHP from generating undefined index notices.This change improves code robustness by handling potentially missing array keys gracefully. It's a good use of PHP's null coalescing operator which is more concise than the equivalent
isset()check.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
emhttp/plugins/dynamix/include/Wireless.php(1 hunks)emhttp/plugins/dynamix/scripts/netconfig(2 hunks)
🔇 Additional comments (2)
emhttp/plugins/dynamix/scripts/netconfig (2)
157-159: Removed conditional check before starting interface with updated configuration.The code now unconditionally executes the start command, creates the network configuration file, and updates WireGuard regardless of whether the port exists.
This change simplifies the control flow but removes a safeguard. Ensure this behavior is intentional and aligns with how the networking stack handles non-existent interfaces.
The underlying system commands might fail gracefully when an interface doesn't exist, but since all errors are redirected to /dev/null, they won't be visible in logs, which might make troubleshooting more difficult.
93-93:Details
❓ Verification inconclusive
Removed conditional check for port existence before stopping interface.
The code now unconditionally executes the stop command for a network interface regardless of whether the port exists.
This change appears to fix a regression by ensuring the stop command always executes. However, verify this behavior is intended since it will attempt to stop interfaces even if they don't physically exist.
🏁 Script executed:
#!/bin/bash # Check if the port_exists function is used elsewhere to verify interfaces rg -A 2 -B 2 "port_exists" --type phpLength of output: 37
Attention: Confirm intended behavior for unconditionally stopping network interfaces
The code now removes any check for the interface’s existence and always executes the stop command:
exec("/etc/rc.d/rc.inet1 ".escapeshellarg("{$ifname}_stop")." >/dev/null");Our repository search for any occurrence of
port_exists(both in PHP and shell scripts) returned no results, suggesting that the prior conditional check wasn’t being used elsewhere. Please verify manually that invoking the stop command on non-existent interfaces is acceptable and won’t lead to any unintended side effects.
2 participants
Summary by CodeRabbit