Paths

Table of Contentst

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

	F132260322: D12860.diff
	Wed, Oct 15, 6:58 AM

	Unknown Object (File)
	Sat, Oct 4, 12:12 AM

	Unknown Object (File)
	Thu, Oct 2, 1:15 AM

	Unknown Object (File)
	Sat, Sep 20, 11:40 PM

	Unknown Object (File)
	Sat, Sep 20, 3:31 PM

	Unknown Object (File)
	Fri, Sep 19, 7:36 AM

	Unknown Object (File)
	Fri, Sep 19, 3:05 AM

	Unknown Object (File)
	Wed, Sep 17, 11:08 PM

View All 96 Files

Subscribers

gtzalik_amazon.com

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.

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
1400	This one is not necessary, `rss_support` is already a bool.
1404	As above, `frag` is bool.
1405	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.
1553–1564	As above, `l3_csum_err` and `l4_csum_err` are already bools.

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
2207	Please correct the condition or leave the original version, if running field is boolean.
2442	Please align lines 2427 and 2428
2967	is_drbr_empty is should be treated as boolean

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

Path

Size

sys/

dev/

ena/

2 lines

456 lines

39 lines

Diff 34978

sys/dev/ena/ena.h

Loading...

sys/dev/ena/ena.c

Loading...

sys/dev/ena/ena_sysctl.c

Loading...