10163 ufs_fsck: bitwise comparison always evaluates to false

Review Request #1333 — Created Dec. 25, 2018 and submitted

tsoome
illumos-gate
10163
7214b03...
general
pass2.c: In function 'pass2':
pass2.c:243:42: error: bitwise comparison always evaluates to false [-Werror=tautological-compare]
   if ((statemap[inp->i_number] & STMASK) == DCLEAR ||
                                          ^~
cc1: all warnings being treated as errors


  • 0
  • 0
  • 3
  • 1
  • 4
Description From Last Updated
domag02
  1. Whitespace nits and c-style.
  2. usr/src/cmd/fs.d/ufs/fsck/pass2.c (Diff revision 1)
     
     
    Whitespace followed by tab.
  3. usr/src/cmd/fs.d/ufs/fsck/pass2.c (Diff revision 1)
     
     
    Whitespace followed by tab (+ in the next 6 lines).
  4. usr/src/cmd/fs.d/ufs/fsck/pass2.c (Diff revision 1)
     
     
    "If one arm of an if-else statement contains braces, all arms should contain braces."
  5. 
      
tsoome
rm
  1. 
      
  2. usr/src/cmd/fs.d/ufs/fsck/pass2.c (Diff revision 2)
     
     
    How did you decide to also check for INCLEAR here as opposed to changing the right hand side to only be DSTATE?
    1. 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. Thanks for the analysis here. Would you mind making sure this ends up in the bug?

  3. 
      
rm
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...