10163 ufs_fsck: bitwise comparison always evaluates to false

Review Request #1333 — Created Dec. 25, 2018 and submitted Feb. 21, 2019, 11:39 a.m.

Information
Owner:	tsoome
Repository:	illumos-gate
Branch:
Bugs:	10163
Depends On:
Commit:	7214b03...
Reviewers
Groups:	general
People:

Description

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

Testing Done

Issues

0
0
3
1
All Issues: 4

Description	From	Last Updated
Whitespace followed by tab.	domag02	Dec. 26, 2018, 8:42 a.m.
Whitespace followed by tab (+ in the next 6 lines).	domag02	Dec. 26, 2018, 8:42 a.m.
"If one arm of an if-else statement contains braces, all arms should contain braces."	domag02	Dec. 26, 2018, 8:48 a.m.
How did you decide to also check for INCLEAR here as opposed to changing the right hand side to only ...	rm	Feb. 2, 2019, 8:43 a.m.

Ship it!

```
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."

Change Summary:

cstyle cleanup

Commit:

-	700482138822713a13b41110735c1f6dfd28057a
+	7214b03aa07237ce0dd5894b79534de86aca20d9

Diff:

Revision 2 (+35 -34)

Show changes

usr/src/cmd/fs.d/ufs/fsck/pass2.c

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?

tsoome 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.

rm 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!

```
Ship It!
```

You have a pending review.

Review Board 3.0.18

10163 ufs_fsck: bitwise comparison always evaluates to false

Change Summary:

Commit:

Diff:

Status: Closed (submitted)