cleanup: fix -Wmissing-braces warning in net_adapter_linux#134
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe pull request updates Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 3/5 reviews remaining, refill in 22 minutes and 8 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
net_adapter_linux.c (1)
45-73:⚠️ Potential issue | 🟠 MajorUse an explicit socket error check (
fd >= 0) instead ofif (fd)
if (fd)is incorrect forsocket():-1means failure, but0is a valid descriptor. Withfd == 0, this code wrongly skips all ioctl calls and also skipsclose(fd), causing incorrect behavior and resource leaks.Suggested fix
- int fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_IP); - if (fd) { + int fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_IP); + if (fd >= 0) { struct ifreq ifr; memset(&ifr, 0, sizeof(ifr)); @@ close(fd); } else { strncat(adapter.name, ifa->ifa_name, sizeof(adapter.name) - 1); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@net_adapter_linux.c` around lines 45 - 73, The socket success check is wrong: replace the `if (fd)` test after `int fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_IP);` with an explicit `fd >= 0` check so valid descriptor 0 is treated as success; ensure all ioctl calls and the `close(fd)` occur inside that `fd >= 0` branch and that the failure path (when socket returns -1) still falls back to copying `ifa->ifa_name` into `adapter.name`. Update uses of `fd`, `socket(...)`, and `close(fd)` accordingly to avoid skipping ioctls or leaking the descriptor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@net_adapter_linux.c`:
- Around line 45-73: The socket success check is wrong: replace the `if (fd)`
test after `int fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_IP);` with an explicit
`fd >= 0` check so valid descriptor 0 is treated as success; ensure all ioctl
calls and the `close(fd)` occur inside that `fd >= 0` branch and that the
failure path (when socket returns -1) still falls back to copying
`ifa->ifa_name` into `adapter.name`. Update uses of `fd`, `socket(...)`, and
`close(fd)` accordingly to avoid skipping ioctls or leaking the descriptor.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #134 +/- ##
==========================================
+ Coverage 58.57% 60.93% +2.35%
==========================================
Files 32 33 +1
Lines 2607 2813 +206
Branches 526 527 +1
==========================================
+ Hits 1527 1714 +187
- Misses 745 787 +42
+ Partials 335 312 -23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The first member of struct ifreq is a union, so clang's
-Wmissing-braces flagged `struct ifreq ifr = {0};`. Zero-initialize
with memset to match the surrounding style.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nmoinvaz
force-pushed
the
nm/master/missing-braces-warning
branch
from
121e5de to
e2f33dd
Compare
nmoinvaz
changed the title
| struct ifreq ifr; | ||
| memset(&ifr, 0, sizeof(ifr)); |
There was a problem hiding this comment.
Why not this change as it wanted?
| struct ifreq ifr; | |
| memset(&ifr, 0, sizeof(ifr)); | |
| struct ifreq ifr = {{0}}; |
This seems also valid in C
| struct ifreq ifr; | |
| memset(&ifr, 0, sizeof(ifr)); | |
| struct ifreq ifr = {{}}; |
3 participants
Summary
-Wmissing-bracesflaggedstruct ifreq ifr = {0};because the first member ofstruct ifreqis a union.memset-based zero initialization, matching the existing style used four lines above foradapter.Summary by CodeRabbit