Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-3710

libhdfs misuses O_RDONLY/WRONLY/RDWR

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-alpha
    • Fix Version/s: 2.0.2-alpha
    • Component/s: libhdfs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The O_RDONLY / O_WRONLY / O_RDWR macros in fcntl.h are not a bitmask; they are an enum stored in the low bits of the flag word. The proper way to use them is

      if ((flags & O_ACCMODE) == O_RDONLY)
      

      rather than

      if ((flags & O_RDONLY) == 0)
      

      There are many examples of this misuse in hdfs.c.

      As a result of this incorrect testing, erroneous code may be accepted without error and correct code might not work correctly.

      1. hdfs-3710-3.txt
        4 kB
        Andy Isaacson
      2. hdfs-3710-2.txt
        4 kB
        Andy Isaacson
      3. hdfs-3710.txt
        4 kB
        Andy Isaacson

        Issue Links

          Activity

          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12537629/hdfs-3710.txt
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.datanode.TestBlockRecovery
          org.apache.hadoop.hdfs.server.namenode.metrics.TestNameNodeMetrics

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2890//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2890//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12537629/hdfs-3710.txt against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.datanode.TestBlockRecovery org.apache.hadoop.hdfs.server.namenode.metrics.TestNameNodeMetrics +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2890//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2890//console This message is automatically generated.
          Hide
          Colin Patrick McCabe added a comment -

          Looks good.

          The only thing that could make it better is adding a line or two to the unit test at:
          ./hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c

          testing that passing mode = 0 to hdfsOpen is no longer incorrectly accepted.

          Show
          Colin Patrick McCabe added a comment - Looks good. The only thing that could make it better is adding a line or two to the unit test at: ./hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c testing that passing mode = 0 to hdfsOpen is no longer incorrectly accepted.
          Hide
          Colin Patrick McCabe added a comment -

          er, meant to write "passing mode = 3 to hdfsOpen is no longer incorrectly accepted."

          Show
          Colin Patrick McCabe added a comment - er, meant to write "passing mode = 3 to hdfsOpen is no longer incorrectly accepted."
          Hide
          Andy Isaacson added a comment -

          Add a test that invalid flags value is disallowed.

          Also tweak errno on invalid flags; return EINVAL rather than ENOTSUP.

          Show
          Andy Isaacson added a comment - Add a test that invalid flags value is disallowed. Also tweak errno on invalid flags; return EINVAL rather than ENOTSUP.
          Hide
          Colin Patrick McCabe added a comment -

          +1 lgtm

          Show
          Colin Patrick McCabe added a comment - +1 lgtm
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12537738/hdfs-3710-2.txt
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.TestDatanodeBlockScanner

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2896//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2896//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12537738/hdfs-3710-2.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.TestDatanodeBlockScanner +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2896//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2896//console This message is automatically generated.
          Hide
          Andy Isaacson added a comment -

          Sync patch to recent hdfs changes; fix wording of one error message.

          Show
          Andy Isaacson added a comment - Sync patch to recent hdfs changes; fix wording of one error message.
          Hide
          Colin Patrick McCabe added a comment -

          +1

          Show
          Colin Patrick McCabe added a comment - +1
          Hide
          Aaron T. Myers added a comment -

          +1 pending Jenkins.

          Show
          Aaron T. Myers added a comment - +1 pending Jenkins.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12539890/hdfs-3710-3.txt
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2971//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2971//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12539890/hdfs-3710-3.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2971//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2971//console This message is automatically generated.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #2564 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2564/)
          HDFS-3710. libhdfs misuses O_RDONLY/WRONLY/RDWR. Contributed by Andy Isaacson. (Revision 1370898)

          Result = SUCCESS
          atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1370898
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2564 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2564/ ) HDFS-3710 . libhdfs misuses O_RDONLY/WRONLY/RDWR. Contributed by Andy Isaacson. (Revision 1370898) Result = SUCCESS atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1370898 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #2629 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2629/)
          HDFS-3710. libhdfs misuses O_RDONLY/WRONLY/RDWR. Contributed by Andy Isaacson. (Revision 1370898)

          Result = SUCCESS
          atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1370898
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2629 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2629/ ) HDFS-3710 . libhdfs misuses O_RDONLY/WRONLY/RDWR. Contributed by Andy Isaacson. (Revision 1370898) Result = SUCCESS atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1370898 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c
          Hide
          Aaron T. Myers added a comment -

          I've just committed this to trunk and branch-2.

          Thanks a lot for the contribution, Andy.

          Show
          Aaron T. Myers added a comment - I've just committed this to trunk and branch-2. Thanks a lot for the contribution, Andy.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #2584 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2584/)
          HDFS-3710. libhdfs misuses O_RDONLY/WRONLY/RDWR. Contributed by Andy Isaacson. (Revision 1370898)

          Result = FAILURE
          atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1370898
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2584 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2584/ ) HDFS-3710 . libhdfs misuses O_RDONLY/WRONLY/RDWR. Contributed by Andy Isaacson. (Revision 1370898) Result = FAILURE atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1370898 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1130 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1130/)
          HDFS-3710. libhdfs misuses O_RDONLY/WRONLY/RDWR. Contributed by Andy Isaacson. (Revision 1370898)

          Result = SUCCESS
          atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1370898
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1130 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1130/ ) HDFS-3710 . libhdfs misuses O_RDONLY/WRONLY/RDWR. Contributed by Andy Isaacson. (Revision 1370898) Result = SUCCESS atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1370898 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1162 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1162/)
          HDFS-3710. libhdfs misuses O_RDONLY/WRONLY/RDWR. Contributed by Andy Isaacson. (Revision 1370898)

          Result = FAILURE
          atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1370898
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1162 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1162/ ) HDFS-3710 . libhdfs misuses O_RDONLY/WRONLY/RDWR. Contributed by Andy Isaacson. (Revision 1370898) Result = FAILURE atm : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1370898 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c

            People

            • Assignee:
              Andy Isaacson
              Reporter:
              Andy Isaacson
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development