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

Discrepancies/inconsistencies in coding standards, style and examples (Bugzilla Bug 2664) #23

Open
tianocore-issues opened this issue Apr 7, 2020 · 7 comments

Comments

@tianocore-issues
Copy link

This issue was created automatically with bugzilla2github

Bugzilla Bug 2664

Date: 2020-04-07T01:23:18+00:00
From: Sing Wee <<sing.wee>>
To: @mdkinney
CC: @changab, @lgao4, @makubacki, nobody, @bexcran, sing.wee

Last updated: 2022-11-15T22:16:39+00:00

@tianocore-issues
Copy link
Author

Comment 12080

Date: 2020-04-07 01:23:18 +0000
From: Sing Wee <<sing.wee>>

  • Industry Specification: ---
  • Release Observed: EDK II Master
  • Releases to Fix: EDK II Master
  • Target OS: ---
  • Bugzilla Assignee(s): @mdkinney

There appear to be a number of inconsistencies in the coding standards found at the following link [DRAFT FOR REVIEW [9/18/2019 10:20:55]: https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Specifications#c-coding-standards

  1. 5.2.1.6
  • Horizontal spacing rules say to put a space before an open parenthesis except in the case of ((. I think the extra spaces in the first two lines of this sample after the open parentheses should be removed.
  • Per the coding conventions, there should also be a space between "ShipIt" and the open parenthesis.
  • Should/can the logical '&&' line up after the open parenthesis on the first line per 5.2.2.11?
    ---------8<---------
    while ( ( Code == MEETS_STANDARD)
    && ( Code == FUNCTIONAL))
    {
    ShipIt();
    }
    --------->8---------
  1. 5.7.3.8 example (page 82). Why the space after the '!'?
    ---------8<---------
    if (! EFI_ERROR (Status)) {
    IDoTheWork ();
    }
    return Status;
    --------->8---------

  2. When casting, there are contradictory examples about whether a space should exist before the variable name:
    5.7.2.3 has: ((INTN)foo >= 0) <----- No space
    5.7.2.4 has: (EFI_PHYSICAL_ADDRESS)(UINTN) Handle <----- Yes space

  3. In 5.4.1.2.1 example, I believe the spaces after open parentheses in function calls and the for-loop should be removed.

  4. When it comes to multi-line predicate expressions, there are inconsistent examples of where the logical operators should go (at the end or beginning of the line).

    In 5.7.2.3: Example shows && at the end of the line.
    In 5.2.7.4: Example shows && a the beginning of the line.

  5. When it comes to multiline predicate expressions, there are inconsistent examples of indentation on the second (and subsequent) lines of such expressions. (I personally like the example in 5.7.2.3, which follows guidance of rule 5.2.2.11. the other examples do not follow this guidance.)

    In 5.2.1.6: Indent is 2 spaces.
    In 5.7.2.3: Line up parentheses in a way to show depth of parentheses.
    In 5.7.2.4: Line up with one more space than 5.7.2.3.

  6. 5.7.2.4 examples do not follow guidance of 3.2.4, which says you should use additional parentheses instead of relying on operator precedence in predicate expressions.

@tianocore-issues
Copy link
Author

Comment 12089

Date: 2020-04-07 21:35:24 +0000
From: nobody <>

Mike: can you check the issues in code style document?

@tianocore-issues
Copy link
Author

Comment 14539

Date: 2020-12-09 15:34:21 +0000
From: @bexcran

I've submitted patches to fix some of these, including the spaces after opening parentheses, and the space after a unary operator.

@tianocore-issues
Copy link
Author

Comment 17435

Date: 2021-10-02 03:29:25 +0000
From: Sing Wee <<sing.wee>>

Noticed the word "Functioon" in Appendix A. Perhaps we can fix that typo at the same time?

@tianocore-issues
Copy link
Author

Comment 19953

Date: 2022-11-07 08:50:04 +0000
From: @changab

I think we can fix the above inconsistent issues in the version 2.3 release.

@tianocore-issues
Copy link
Author

Comment 19994

Date: 2022-11-10 23:29:38 +0000
From: @mdkinney

I agree that these issues can be reviewed.

Please review commits already made.

And, any code examples in the doc should exactly match the style produced by uncrustify.

Please update with the set of remaining changes after those reviews

@tianocore-issues
Copy link
Author

Comment 20043

Date: 2022-11-15 22:16:39 +0000
From: @lgao4

Include [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant