Whitespace nits and c-style.

usr/src/cmd/fs.d/ufs/fsck/pass2.c (Diff revision 1)

Show all issues

Whitespace followed by tab.

usr/src/cmd/fs.d/ufs/fsck/pass2.c (Diff revision 1)

Show all issues

Whitespace followed by tab (+ in the next 6 lines).

usr/src/cmd/fs.d/ufs/fsck/pass2.c (Diff revision 1)

Show all issues

"If one arm of an if-else statement contains braces, all arms should contain braces."

usr/src/cmd/fs.d/ufs/fsck/pass2.c (Diff revision 2)

Show all issues

How did you decide to also check for INCLEAR here as opposed to changing the right hand side to only be DSTATE?

1. 

   Toomas Soome Jan. 28, 2019, 11:07 a.m.

   The whole idea why this check is in place seems to come from phase1:
           if (isdir && (dp->di_blocks == 0)) {
                   /*
                    * INCLEAR will cause passes 2 and 3 to skip it.
                    */
                   (void) printf("DIR WITH ZERO BLOCKS  I=%d\n", inumber);
                   statemap[inumber] = DCLEAR;
                   add_orphan_dir(inumber);
           }
   
   and
           if (isdir) {
                   /*
                    * INCLEAR makes passes 2 & 3 skip it.
                    */
                   statemap[inumber] = DCLEAR;
                   add_orphan_dir(inumber);
                   cacheino(dp, inumber);
           }
   
   
   But since this loop is about directories, it seems the check should actually just test if statemap[inp->i_number] == DCLEAR. However, we also do get state set as:
   if (dp->di_size == 0) {
                           /*
                            * INCLEAR means it will be ignored by passes 2 & 3.
                            */
                           if ((dp->di_mode & IFMT) == IFDIR)
                                   (void) printf("ZERO-LENGTH DIR  I=%d\n",
                                       inumber);
                           else
                                   (void) printf("ZERO-LENGTH ATTRDIR  I=%d\n",
                                       inumber);
                           add_orphan_dir(inumber);
                           flags |= INCLEAR;
                           flags &= ~INZLINK;      /* It will be cleared anyway */
                   }
                   statemap[inumber] = DSTATE | flags;
   
   
   So the intent seems to be to trigger those ignores mentioned above, and therefore the INCLEAR checks have to be there.

2. 

   Robert Mustacchi Feb. 21, 2019, 2:47 a.m.

   Thanks for the analysis here. Would you mind making sure this ends up in the bug?

Ship It!