jimzah/openafs
1
0
Fork 0
mirror of https://git.openafs.org/openafs.git synced 2025-01-31 13:38:01 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

LINUX: Use bitwise & for f_flags test

Browse Source 
This code is clearly supposed to be masking f_flags with O_ACCMODE, and
so should be using a bitwise &, not the boolean &&.  Clang complains
about this, causing a warning (which breaks the build with
--enable-checking):

error: use of logical '&&' with constant operand [-Werror,
        -Wconstant-logical-operand]
        } else if ((file->f_flags && O_ACCMODE) != O_WRONLY) {
                                  ^  ~~~~~~~~~
.../osi_vnodeops.c:3192:28: note: use '&' for a
        bitwise operation

For the current code without this commit the behavior of this
check is as follows:

   When f_flags is:         The check is:
   ====================     =============
   O_RDONLY                 True  (as expected)
   O_WRONLY                 False (as expected)
   O_RDWR                   False (incorrect)
   has some non-O_ACCMODE   False (incorrect)
    bit is set

The incorrect check doesn't cause any problems for overall correctness,
but it does mean that in those cases afs_linux_fillpage will not be
called and that partially-written pages will be left out of date and any
reader will need to fill the page again.

Fix this by using the bitwise &.

There is also an out of date link to the Linux documentation.  Update
the comment to point to the current documentation that is in the Linux
source tree.

Reviewed-on: https://gerrit.openafs.org/14903
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 8d9545008240d7f285b140503e432f53f41e0724)

Change-Id: Iae459ffe87df9a21a90587c02a1ee2c20b360d33
Reviewed-on: https://gerrit.openafs.org/15129
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
This commit is contained in:
Cheyenne Wills 2022-03-07 18:35:03 -07:00 committed by Stephan Wiesand
parent c3f31cc9c3
commit e3edd8c43e
1 changed files with 6 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/afs/LINUX/osi_vnodeops.c
View File

@ -3438,9 +3438,11 @@ afs_linux_prepare_write(struct file *file, struct page *page, unsigned from,
unsigned to) unsigned to)
{ {
/* http://kerneltrap.org/node/4941 details the expected behaviour of /*
* prepare_write. Essentially, if the page exists within the file, * Linux's Documentation/filesystems/vfs.txt (.rst) details the expected
* and is not being fully written, then we should populate it. * behaviour of prepare_write (prior to 2.6.28) and write_begin (2.6.28).
* Essentially, if the page exists within the file, and is not being fully
* written, then we should populate it.
*/ */
if (!PageUptodate(page)) { if (!PageUptodate(page)) {
@ -3457,7 +3459,7 @@ afs_linux_prepare_write(struct file *file, struct page *page, unsigned from,
SetPageChecked(page); SetPageChecked(page);
/* Is the page readable, if it's wronly, we don't care, because we're /* Is the page readable, if it's wronly, we don't care, because we're
* not actually going to read from it ... */ * not actually going to read from it ... */
} else if ((file->f_flags && O_ACCMODE) != O_WRONLY) { } else if ((file->f_flags & O_ACCMODE) != O_WRONLY) {
/* We don't care if fillpage fails, because if it does the page /* We don't care if fillpage fails, because if it does the page
* won't be marked as up to date * won't be marked as up to date
*/ */