Page MenuHomeFreeBSD
Differential D12860

Refactor style of the ENA driver
ClosedPublic
Actions

Authored by mk_semihalf.com on Oct 31 2017, 1:39 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, May 17, 7:08 PM
Unknown Object (File)
Dec 20 2023, 6:36 AM
Unknown Object (File)
Nov 25 2023, 11:41 PM
Unknown Object (File)
Nov 23 2023, 3:34 PM
Unknown Object (File)
Nov 23 2023, 6:21 AM
Unknown Object (File)
Nov 16 2023, 7:22 AM
Unknown Object (File)
Sep 6 2023, 6:44 PM
Unknown Object (File)
Aug 15 2023, 6:28 AM
View All 19 Files
Subscribers
gtzalik_amazon.com
imp

Details

Reviewers
byenduri_gmail.com
rlibby
mw
Commits
rS325580: Refactor style of the ENA driver
Summary
  • Change all conditional checks in "if" statement to boolean expressions
  • Initialzie variables with too complex values outside the declaration
  • Fix indentations
  • Move code associated with sysctls to ena_sysctl.c file
  • For consistency, remove unnecesary "return" from void functions
  • Use if_getdrvflags() function instead of accesing variable directly

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

mk_semihalf.com created this revision.Oct 31 2017, 1:39 PM
Herald added a subscriber: imp. · View Herald TranscriptOct 31 2017, 1:39 PM
mk_semihalf.com added a parent revision: D12859: Fix error handling in the ENA driver and lock drbr_free() call.Oct 31 2017, 1:40 PM
mk_semihalf.com added a child revision: D12861: Cover ENA driver code with branch predictioning statements.
rlibby added a comment.Nov 1 2017, 7:23 AM

Most of these changes look to be in the right direction. I don't think evaluating if a bool == true or == false is necessary. In some places parentheses have been added which are not strictly needed, I don't feel strongly about those either way. I did not evaluate for completeness.

sys/dev/ena/ena.c
1409

This one is not necessary, rss_support is already a bool.

1413

As above, frag is bool.

1414

style(9) discourages excess parentheses when the parsing is not confusing. I don't have a complaint if you prefer to parenthesize this, but FYI it is not required by style.

1566–1577

As above, l3_csum_err and l4_csum_err are already bools.

mk_semihalf.com updated this revision to Diff 34672.Nov 2 2017, 2:48 PM

The boolean variables are no longer compared explicitly to true/false.

mk_semihalf.com marked 3 inline comments as done.Nov 2 2017, 2:49 PM
mk_semihalf.com mentioned this in D12861: Cover ENA driver code with branch predictioning statements.
mw added inline comments.Nov 8 2017, 11:26 PM
sys/dev/ena/ena.c
2220

Please correct the condition or leave the original version, if running field is boolean.

2455

Please align lines 2427 and 2428

2980

is_drbr_empty is should be treated as boolean

mk_semihalf.com updated this revision to Diff 34978.Nov 9 2017, 10:58 AM
mk_semihalf.com marked 3 inline comments as done.

Fix style issues and invalid conditional check

mw accepted this revision.Nov 9 2017, 11:27 AM
This revision is now accepted and ready to land.Nov 9 2017, 11:27 AM
Closed by commit rS325580: Refactor style of the ENA driver (authored by mw). · Explain WhyNov 9 2017, 11:57 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
dev/
ena/
2 lines
458 lines
39 lines

Diff 34672

View Options

sys/dev/ena/ena.h

Loading...
View Options

sys/dev/ena/ena.c

Loading...
View Options

sys/dev/ena/ena_sysctl.c

Loading...