Repository navigation
Conversation
$interface is used for creating files. $ifxname is used to reference the interace directly. This is an important distinction for OS who can put path separators or invalid path characters into the interface name.
Walkthroughmake_env() now checks return values from print_string(...) when creating filesystem-safe ChangesInterface name escaping error handling in make_env()
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)src/script.csrc/script.c:48:10: fatal error: 'config.h' file not found ... [truncated 658 characters] ... Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/script.c (1)
62-64:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
ifxnameto the exported script-parameter list.
make_env()now exportsifxname, butif_printoptions()won’t advertise it becauseif_paramsis missing the entry.Suggested update
static const char *const if_params[] = { "interface", "protocol", "reason", "pid", "ifcarrier", "ifmetric", "ifwireless", "ifflags", "ssid", - "profile", "interface_order", NULL }; + "profile", "ifxname", "interface_order", NULL };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/script.c` around lines 62 - 64, The exported parameter list array if_params is missing "ifxname", so if_printoptions won't advertise the new variable even though make_env() exports it; update the static const char *const if_params[] initializer to include "ifxname" among the other parameter names (e.g., alongside "interface", "protocol", "reason", etc.) so the script-parameter list and the existing functions if_printoptions and make_env() remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/script.c`:
- Around line 340-342: Reformat the print_string invocation to satisfy
clang-format: rewrite the call to print_string(if_name, sizeof(if_name),
OT_ESCFILE, ifp->name, strlen(ifp->name)) so the arguments are laid out
according to project style (e.g. each argument on its own indented line or kept
on one line if it fits), ensure the closing parenthesis and semicolon are
aligned properly, and keep the existing variables and the goto eexit path
unchanged; update the call site referencing print_string, if_name,
sizeof(if_name), OT_ESCFILE, ifp->name, and strlen(ifp->name).
---
Outside diff comments:
In `@src/script.c`:
- Around line 62-64: The exported parameter list array if_params is missing
"ifxname", so if_printoptions won't advertise the new variable even though
make_env() exports it; update the static const char *const if_params[]
initializer to include "ifxname" among the other parameter names (e.g.,
alongside "interface", "protocol", "reason", etc.) so the script-parameter list
and the existing functions if_printoptions and make_env() remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/script.c (1)
340-342:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the clang-format violation in the
print_stringcall.This formatting issue was already flagged by CI. Per the pipeline failure, the arguments should be reformatted so that
ifp->nameappears on the same line asOT_ESCFILE,.🔧 Suggested formatting fix
- if (print_string(if_name, sizeof(if_name), OT_ESCFILE, - ifp->name, strlen(ifp->name)) == -1) + if (print_string(if_name, sizeof(if_name), OT_ESCFILE, ifp->name, + strlen(ifp->name)) == -1) goto eexit;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/script.c` around lines 340 - 342, The call to print_string is split across lines causing a clang-format violation; reformat the argument list so the OT_ESCFILE and the following argument ifp->name appear on the same line. Locate the print_string invocation that passes if_name, sizeof(if_name), OT_ESCFILE and ifp->name, and collapse the line break so it reads with OT_ESCFILE, ifp->name on the same line (preserving the trailing strlen(ifp->name)) to satisfy the formatter.Source: Pipeline failures
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/script.c`:
- Around line 405-407: The call to print_string is misformatted; update the
invocation of print_string (the call using variables if_name, sizeof(if_name),
OT_ESCFILE, rt->rt_ifp->name, strlen(rt->rt_ifp->name)) to match the project's
multi-line formatting style used elsewhere (same pattern as lines 340-342) so
clang-format passes — break arguments across lines with the function name and
opening parenthesis on the first line and each argument on its own indented
line, keeping the trailing ) and the == -1 check aligned with the goto eexit;.
- Around line 345-347: Reformat the print_string call to match the existing
multi-line argument layout used on lines 340–342: break the arguments across
lines the same way (one argument per line aligned with the opening parenthesis)
and keep the same tokens (print_string, if_name, sizeof(if_name), OT_ESCSTRING,
ifp->name, strlen(ifp->name)); preserve the error check and the goto eexit
behavior but adjust whitespace/line breaks to conform to the project's
clang-format style.
---
Duplicate comments:
In `@src/script.c`:
- Around line 340-342: The call to print_string is split across lines causing a
clang-format violation; reformat the argument list so the OT_ESCFILE and the
following argument ifp->name appear on the same line. Locate the print_string
invocation that passes if_name, sizeof(if_name), OT_ESCFILE and ifp->name, and
collapse the line break so it reads with OT_ESCFILE, ifp->name on the same line
(preserving the trailing strlen(ifp->name)) to satisfy the formatter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| if (print_string(if_name, sizeof(if_name), OT_ESCSTRING, | ||
| ifp->name, strlen(ifp->name)) == -1) | ||
| goto eexit; |
There was a problem hiding this comment.
Apply consistent formatting to this print_string call.
This call follows the same formatting pattern as lines 340-342, which violates clang-format rules. For consistency with the project style and to avoid CI failures, apply the same fix here.
🔧 Suggested formatting fix
- if (print_string(if_name, sizeof(if_name), OT_ESCSTRING,
- ifp->name, strlen(ifp->name)) == -1)
+ if (print_string(if_name, sizeof(if_name), OT_ESCSTRING, ifp->name,
+ strlen(ifp->name)) == -1)
goto eexit;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (print_string(if_name, sizeof(if_name), OT_ESCSTRING, | |
| ifp->name, strlen(ifp->name)) == -1) | |
| goto eexit; | |
| if (print_string(if_name, sizeof(if_name), OT_ESCSTRING, ifp->name, | |
| strlen(ifp->name)) == -1) | |
| goto eexit; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/script.c` around lines 345 - 347, Reformat the print_string call to match
the existing multi-line argument layout used on lines 340–342: break the
arguments across lines the same way (one argument per line aligned with the
opening parenthesis) and keep the same tokens (print_string, if_name,
sizeof(if_name), OT_ESCSTRING, ifp->name, strlen(ifp->name)); preserve the error
check and the goto eexit behavior but adjust whitespace/line breaks to conform
to the project's clang-format style.
| if (print_string(if_name, sizeof(if_name), OT_ESCFILE, | ||
| rt->rt_ifp->name, strlen(rt->rt_ifp->name)) == -1) | ||
| goto eexit; |
There was a problem hiding this comment.
Apply consistent formatting to this print_string call.
This call follows the same formatting pattern as lines 340-342, which violates clang-format rules. For consistency with the project style and to avoid CI failures, apply the same fix here.
🔧 Suggested formatting fix
- if (print_string(if_name, sizeof(if_name), OT_ESCFILE,
- rt->rt_ifp->name, strlen(rt->rt_ifp->name)) == -1)
+ if (print_string(if_name, sizeof(if_name), OT_ESCFILE, rt->rt_ifp->name,
+ strlen(rt->rt_ifp->name)) == -1)
goto eexit;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (print_string(if_name, sizeof(if_name), OT_ESCFILE, | |
| rt->rt_ifp->name, strlen(rt->rt_ifp->name)) == -1) | |
| goto eexit; | |
| if (print_string(if_name, sizeof(if_name), OT_ESCFILE, rt->rt_ifp->name, | |
| strlen(rt->rt_ifp->name)) == -1) | |
| goto eexit; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/script.c` around lines 405 - 407, The call to print_string is
misformatted; update the invocation of print_string (the call using variables
if_name, sizeof(if_name), OT_ESCFILE, rt->rt_ifp->name,
strlen(rt->rt_ifp->name)) to match the project's multi-line formatting style
used elsewhere (same pattern as lines 340-342) so clang-format passes — break
arguments across lines with the function name and opening parenthesis on the
first line and each argument on its own indented line, keeping the trailing )
and the == -1 check aligned with the goto eexit;.
$interface is used for creating files.
$ifxname is used to reference the interace directly.
This is an important distinction for OS who can put path separators or invalid path characters into the interface name.