Uploaded image for project: 'WiredTiger'
  1. WiredTiger
  2. WT-2093

Use the C "bool" type for true/false function returns

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major - P3
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: WT2.7.0
    • Labels:
      None
    • # Replies:
      8
    • Last comment by Customer:
      true

      Description

      In a recent code review, there was some discussion about when function returns should be compared against an explicit zero vs if (!func()). The answer is if the function is returning a boolean, but WT has mapped that to the int type for portability.

      However, C has had a bool type since C99 via <stdbool.h>, also supported by Visual Studio 2013 and above.

      Investigate converting WT to use explicit bool types to make it clear when expressions need to be compared against zero.

        Issue Links

          Activity

          Hide
          michael.cahill Michael Cahill added a comment -

          During review, I found two bugs by inspection:

          In block_addr.c:

           86 /*
           87  * __wt_block_addr_valid --
           88  *      Return if an address cookie is valid.
           89  */
           90 int
           91 __wt_block_addr_valid(WT_SESSION_IMPL *session,
           92     WT_BLOCK *block, const uint8_t *addr, size_t addr_size, int live)
           93 {
           94         wt_off_t offset;
           95         uint32_t cksum, size;
          ...
          100
          101         /* Crack the cookie. */
          102         WT_RET(__wt_block_buffer_to_addr(block, addr, &offset, &size, &cksum));
          ...
          112
          113         /* Check if it's past the end of the file. */
          114         return (offset + size > block->fh->size ? 0 : 1);
          115 }
          

          Here an error from __wt_block_buffer_to_addr will be treated as a valid address.

          In rec_track.c:

          448 int
          449 __wt_ovfl_reuse_search(WT_SESSION_IMPL *session, WT_PAGE *page,
          450     uint8_t **addrp, size_t *addr_sizep,
          451     const void *value, size_t value_size)
          452 {
          ...
          467         if ((reuse = __ovfl_reuse_skip_search(head, value, value_size)) == NULL)
          468                 return (0);
          469
          ...
          474         if (WT_VERBOSE_ISSET(session, WT_VERB_OVERFLOW))
          475                 WT_RET(__ovfl_reuse_verbose(session, page, reuse, "reclaim"));
          476         return (1);
          

          Here an error from __ovfl_reuse_verbose will be treated as found (which it is) but the error return is lost.

          Show
          michael.cahill Michael Cahill added a comment - During review, I found two bugs by inspection: In block_addr.c : 86 /* 87 * __wt_block_addr_valid -- 88 * Return if an address cookie is valid. 89 */ 90 int 91 __wt_block_addr_valid(WT_SESSION_IMPL *session, 92 WT_BLOCK *block, const uint8_t *addr, size_t addr_size, int live) 93 { 94 wt_off_t offset; 95 uint32_t cksum, size; ... 100 101 /* Crack the cookie. */ 102 WT_RET(__wt_block_buffer_to_addr(block, addr, &offset, &size, &cksum)); ... 112 113 /* Check if it's past the end of the file. */ 114 return (offset + size > block->fh->size ? 0 : 1); 115 } Here an error from __wt_block_buffer_to_addr will be treated as a valid address. In rec_track.c : 448 int 449 __wt_ovfl_reuse_search(WT_SESSION_IMPL *session, WT_PAGE *page, 450 uint8_t **addrp, size_t *addr_sizep, 451 const void *value, size_t value_size) 452 { ... 467 if ((reuse = __ovfl_reuse_skip_search(head, value, value_size)) == NULL) 468 return (0); 469 ... 474 if (WT_VERBOSE_ISSET(session, WT_VERB_OVERFLOW)) 475 WT_RET(__ovfl_reuse_verbose(session, page, reuse, "reclaim")); 476 return (1); Here an error from __ovfl_reuse_verbose will be treated as found (which it is) but the error return is lost.
          Hide
          xgen-internal-githook Githook User added a comment -

          Author:

          {u'username': u'michaelcahill', u'name': u'Michael Cahill', u'email': u'michael.cahill@mongodb.com'}

          Message: WT-2093 Use the C99 bool type to clarify when functions return true/false.
          Branch: develop
          https://github.com/wiredtiger/wiredtiger/commit/5a440d45fc052e5ebde92e0266d0dae93f2c701d

          Show
          xgen-internal-githook Githook User added a comment - Author: {u'username': u'michaelcahill', u'name': u'Michael Cahill', u'email': u'michael.cahill@mongodb.com'} Message: WT-2093 Use the C99 bool type to clarify when functions return true/false. Branch: develop https://github.com/wiredtiger/wiredtiger/commit/5a440d45fc052e5ebde92e0266d0dae93f2c701d
          Hide
          xgen-internal-githook Githook User added a comment -

          Author:

          {u'username': u'agorrod', u'name': u'Alex Gorrod', u'email': u'alexg@wiredtiger.com'}

          Message: WT-2093 Make types match.
          Branch: develop
          https://github.com/wiredtiger/wiredtiger/commit/39d8cef6c1a693327e3ea8a2c7732ddfdb3fbfaf

          Show
          xgen-internal-githook Githook User added a comment - Author: {u'username': u'agorrod', u'name': u'Alex Gorrod', u'email': u'alexg@wiredtiger.com'} Message: WT-2093 Make types match. Branch: develop https://github.com/wiredtiger/wiredtiger/commit/39d8cef6c1a693327e3ea8a2c7732ddfdb3fbfaf
          Hide
          xgen-internal-githook Githook User added a comment -

          Author:

          {u'username': u'michaelcahill', u'name': u'Michael Cahill', u'email': u'michael.cahill@mongodb.com'}

          Message: Merge pull request #2178 from wiredtiger/use-bool

          WT-2093 Use the C99 bool type to clarify when functions return true/false
          Branch: develop
          https://github.com/wiredtiger/wiredtiger/commit/d9391c0df4dc38c8ea571bde4808ced194d7cff0

          Show
          xgen-internal-githook Githook User added a comment - Author: {u'username': u'michaelcahill', u'name': u'Michael Cahill', u'email': u'michael.cahill@mongodb.com'} Message: Merge pull request #2178 from wiredtiger/use-bool WT-2093 Use the C99 bool type to clarify when functions return true/false Branch: develop https://github.com/wiredtiger/wiredtiger/commit/d9391c0df4dc38c8ea571bde4808ced194d7cff0
          Hide
          xgen-internal-githook Githook User added a comment -

          Author:

          {u'username': u'keithbostic', u'name': u'Keith Bostic', u'email': u'keith@wiredtiger.com'}

          Message: WT-2093: __wt_block_addr_valid() didn't distinguish between a valid
          address and an underlying error cracking the address cookie. Change
          the name to __wt_block_addr_invalid and then return 0 for "is-valid"
          and an error code for any invalid blocks.
          Branch: develop
          https://github.com/wiredtiger/wiredtiger/commit/52eceffbc938e577a21954c03a986f66670f1ac3

          Show
          xgen-internal-githook Githook User added a comment - Author: {u'username': u'keithbostic', u'name': u'Keith Bostic', u'email': u'keith@wiredtiger.com'} Message: WT-2093 : __wt_block_addr_valid() didn't distinguish between a valid address and an underlying error cracking the address cookie. Change the name to __wt_block_addr_invalid and then return 0 for "is-valid" and an error code for any invalid blocks. Branch: develop https://github.com/wiredtiger/wiredtiger/commit/52eceffbc938e577a21954c03a986f66670f1ac3
          Hide
          xgen-internal-githook Githook User added a comment -

          Author:

          {u'username': u'keithbostic', u'name': u'Keith Bostic', u'email': u'keith@wiredtiger.com'}

          Message: WT-2093: __wt_ovfl_reuse_search() returned 0/1 for "overflow record
          found/not-found", which could discard real error values from underlying
          functions it called. Instead of returning found/not-found, return a
          standard 0/error value, and use the stored address of any found overflow
          record as the found/not-found status.
          Branch: develop
          https://github.com/wiredtiger/wiredtiger/commit/f0ff426b5c2c15c6dde3259271745e2f2d786784

          Show
          xgen-internal-githook Githook User added a comment - Author: {u'username': u'keithbostic', u'name': u'Keith Bostic', u'email': u'keith@wiredtiger.com'} Message: WT-2093 : __wt_ovfl_reuse_search() returned 0/1 for "overflow record found/not-found", which could discard real error values from underlying functions it called. Instead of returning found/not-found, return a standard 0/error value, and use the stored address of any found overflow record as the found/not-found status. Branch: develop https://github.com/wiredtiger/wiredtiger/commit/f0ff426b5c2c15c6dde3259271745e2f2d786784
          Hide
          xgen-internal-githook Githook User added a comment -

          Author:

          {u'username': u'michaelcahill', u'name': u'Michael Cahill', u'email': u'michael.cahill@mongodb.com'}

          Message: Merge pull request #2185 from wiredtiger/wt-2093-cleanup

          WT-2093 Fix bugs treating error returns as booleans
          Branch: develop
          https://github.com/wiredtiger/wiredtiger/commit/8a8867b859d9b1bdeb00084495bd925828ae51e0

          Show
          xgen-internal-githook Githook User added a comment - Author: {u'username': u'michaelcahill', u'name': u'Michael Cahill', u'email': u'michael.cahill@mongodb.com'} Message: Merge pull request #2185 from wiredtiger/wt-2093-cleanup WT-2093 Fix bugs treating error returns as booleans Branch: develop https://github.com/wiredtiger/wiredtiger/commit/8a8867b859d9b1bdeb00084495bd925828ae51e0
          Hide
          xgen-internal-githook Githook User added a comment -

          Author:

          {u'username': u'michaelcahill', u'name': u'Michael Cahill', u'email': u'michael.cahill@mongodb.com'}

          Message: WT-2093 Use the C99 bool type to clarify when functions return true/false

          Merge pull request #2178 from wiredtiger/use-bool

          (cherry picked from commit d9391c0df4dc38c8ea571bde4808ced194d7cff0)
          Branch: mongodb-3.0
          https://github.com/wiredtiger/wiredtiger/commit/8e212cf8907a5d2462dead47aebcce074dd3c066

          Show
          xgen-internal-githook Githook User added a comment - Author: {u'username': u'michaelcahill', u'name': u'Michael Cahill', u'email': u'michael.cahill@mongodb.com'} Message: WT-2093 Use the C99 bool type to clarify when functions return true/false Merge pull request #2178 from wiredtiger/use-bool (cherry picked from commit d9391c0df4dc38c8ea571bde4808ced194d7cff0) Branch: mongodb-3.0 https://github.com/wiredtiger/wiredtiger/commit/8e212cf8907a5d2462dead47aebcce074dd3c066

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since reply:
                1 year, 39 weeks ago
                Date of 1st Reply: