tools/nxstyle.c: FALSE alarms on right hand aligned comments.

[patacongo]

When ran against any file that has comments to the right of definitions, a false alarm can occur. Such right-hand comments should be vertically aligned, but have not specific alignment requirement with regard to the start of the line or any indentation.

In many casese, there are comments on a separate line are correctly aligned with the other right hand comments. This is necessary for the visually alignment, but results in a false alarm error from nxstyle. Consider the following from include/nuttx/can/can.h:

579   struct can_txfifo_s  cd_xmit;          /* Describes transmit FIFO */
580 #ifdef CONFIG_CAN_TXREADY
581   struct work_s        cd_work;          /* Use to manage can_txready() work */
582 #endif
583                                          /* List of pending RTR requests */
584   struct can_rtrwait_s cd_rtr[CONFIG_CAN_NPENDINGRTR];
585   FAR const struct can_ops_s *cd_ops;    /* Arch-specific operations */

Running nxstyle against that file generates this false alarm:

include/nuttx/can/can.h:583:1: error: Missing blank line after comment

Analysis: Notice that there is no error about the missing blank line BEFORE the comment at 583. That is because there is information that the preceding line is right-hand aligned.

498   bool brhcomment;      /* True: Comment to the right of code */
499   bool prevbrhcmt;      /* True: previous line had comment to the right of code */

The problem is that the state information is not propagated at line 583. Could the solution be as simple as just make sure that brhcomment is set true again when the conditioni of line 583 is encountered?

[patacongo]

I think that the problem is an ordering one in nxstyle.c:

• prevbrhcmt is true if the previous line is a right hand comment (brhcomment)
• It is set at line 661 if the last line was a right hand comment
• The error occurs because prevbrhcmt is not set at line 718 or
• Line 963 and 1395: right hand comment should propagate to a single line comment if single line comment was preceded by a right hand comment with no blank line.

I think that the error occurs because the test of prevbrhcmt at line 718 occurs BEFORE it is set the the current line at lines 963 or 1395

[johannes-nivus]

@patacongo I had again a little fun with nxstyle this afternoon.
I addressed Issue #120 and the problem had been quite simple: The prevbrhcmt had to propagate over preprocessor lines.
Additionally I added checks for the column position of the right of code comments. (And caught one in include/nuttx/can/can.h)
I don't want to file a PR without review (I fear your instant merging powers). So please take a look at
https://github.com/johannes-nivus/incubator-nuttx/tree/nxstyle and review.
Regards, Johannes

[patacongo]

I never look at code outside of the respository. Not interested.

PS: Everyone on the Apache time as instant merging power and use that power frequently.

[patacongo]

The tests that should be performed are:

1. Using a test case, verify that positirves are detected.

2. Check for false negatives/positive regression. I do that with a little script something like:

   #!/bin/sh

   FILELIST=find sched -name *.c
   for file in $FILELIST; do
   tools/nxstyle $file
   done

Then run that before the change like:

./doit.sh 1>before.txt 2>&1

Then rebuild with the modified nxstyle.c and:

../doit.sh 1>after.txt 2>&1

Then compare before.txt with after .txt. They should be identical or, if there are false positives/negatives in before.txt, then after.txt might have fixed those.