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

ifup-routes: print warning when ADDRESS0 entry is missing #475

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

suresh2514
Copy link
Contributor

When static routes are added via route- file, the ifup-routes script will skip adding routes if entries not starting with ADDRESS0.

Good to print a warning so that users are aware of this.

When static routes are added via route-<interface> file, the ifup-routes
script will skip adding routes if entries not starting with ADDRESS0.

Good to print a warning so that users are aware of this.
@@ -14,6 +14,13 @@
handle_file () {
. $1
routenum=0

# print a warning if ADDRESS is not starting at ADDRESS0
if [ "x$(eval echo '$'ADDRESS$routenum)x" == "xx" ]; then

Check warning

Code scanning / shellcheck

Avoid x-prefix in comparisons as it no longer serves a purpose. Warning

Avoid x-prefix in comparisons as it no longer serves a purpose.
Copy link
Member

@jamacku jamacku left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@mergify mergify bot merged commit 72a4642 into fedora-sysv:main Feb 7, 2024
7 of 13 checks passed
@@ -14,6 +14,13 @@ MATCH='^[[:space:]]*(\#.*)?$'
handle_file () {
. $1
routenum=0

# print a warning if ADDRESS is not starting at ADDRESS0
if [ "x$(eval echo '$'ADDRESS$routenum)x" == "xx" ]; then
Copy link
Member

@lnykryn lnykryn Feb 7, 2024

Choose a reason for hiding this comment

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

Looks, fine but two small nitpicks for the future. Since the routenum is always 0, the whole eval shenanigan is unnecessary. Simple [ -z "$ADDRESS0" ] would be much nicer.

# print a warning if ADDRESS is not starting at ADDRESS0
if [ "x$(eval echo '$'ADDRESS$routenum)x" == "xx" ]; then
net_log $"Route entries are not sequentially ordered. Skipping $1 file" warning
exit 1
Copy link
Member

Choose a reason for hiding this comment

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

THis can potentially change the behaviour.

@suresh2514
Copy link
Contributor Author

Hi all,

Thanks for the review and for accepting the patch.

regards

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.

3 participants