Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-11849

Fix storage/innobase/* compile warnings

Details

    Attachments

      Issue Links

        Activity

          jplindst Jan Lindström (Inactive) added a comment - http://lists.askmonty.org/pipermail/commits/2017-January/010472.html
          jplindst Jan Lindström (Inactive) added a comment - Windows: http://lists.askmonty.org/pipermail/commits/2017-January/010473.html

          I am not happy to see so many new type casts. Is there any other solution?
          Please use static_cast<ulint>(xxx) or ulint(xxx) instead of (ulint)(xxx).

          The following is ignoring the high-order 32 bits of the field on 32-bit systems:

          		compression_alg = (ulint)mach_read_from_8(buf+FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION);
          

          Maybe we should use mach_read_from_4() from offset +4 instead?
          The following ought to be introducing a warning for an unused variable in non-debug builds:

          diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
          index a5fd7788af2..bc375f20e9b 100644
          --- a/storage/innobase/handler/ha_innodb.cc
          +++ b/storage/innobase/handler/ha_innodb.cc
          @@ -5508,10 +5508,7 @@ innobase_kill_query(
           			trx_mutex_taken = true;
           		}
           
          -#ifdef UNIV_DEBUG
          -		dberr_t err =
          -#endif
          -		lock_trx_handle_wait(trx, true, true);
          +		dberr_t err = lock_trx_handle_wait(trx, true, true);
           
           		ut_ad(err == DB_SUCCESS || err == DB_LOCK_WAIT
           		      || err == DB_DEADLOCK);
          

          Maybe it would be better to remove the MY_ATTRIBUTE((warn_unused_result)) if that was the reason for this change, or to change the API otherwise.
          Please build with both debug and non-debug. Maybe you could submit the patches to buildbot and examine the warnings on every platform?

          marko Marko Mäkelä added a comment - I am not happy to see so many new type casts. Is there any other solution? Please use static_cast<ulint>(xxx) or ulint(xxx) instead of (ulint)(xxx). The following is ignoring the high-order 32 bits of the field on 32-bit systems: compression_alg = (ulint)mach_read_from_8(buf+FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION); Maybe we should use mach_read_from_4() from offset +4 instead? The following ought to be introducing a warning for an unused variable in non-debug builds: diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index a5fd7788af2..bc375f20e9b 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -5508,10 +5508,7 @@ innobase_kill_query( trx_mutex_taken = true; } -#ifdef UNIV_DEBUG - dberr_t err = -#endif - lock_trx_handle_wait(trx, true, true); + dberr_t err = lock_trx_handle_wait(trx, true, true); ut_ad(err == DB_SUCCESS || err == DB_LOCK_WAIT || err == DB_DEADLOCK); Maybe it would be better to remove the MY_ATTRIBUTE((warn_unused_result)) if that was the reason for this change, or to change the API otherwise. Please build with both debug and non-debug. Maybe you could submit the patches to buildbot and examine the warnings on every platform?

          Buildbot results bb-10.2-mdev11849, I can try to avoid some of the casts by changing the variable type, but that does not work for all of these.

          jplindst Jan Lindström (Inactive) added a comment - Buildbot results bb-10.2-mdev11849, I can try to avoid some of the casts by changing the variable type, but that does not work for all of these.

          commit 71495a1748784a887f42888a2a7b8cac5e088ff6
          Author: Jan Lindström <jan.lindstrom@mariadb.com>
          Date: Wed Jan 25 10:11:37 2017 +0200

          MDEV-11849: Fix storage/innobase/* compile warnings

          jplindst Jan Lindström (Inactive) added a comment - commit 71495a1748784a887f42888a2a7b8cac5e088ff6 Author: Jan Lindström <jan.lindstrom@mariadb.com> Date: Wed Jan 25 10:11:37 2017 +0200 MDEV-11849 : Fix storage/innobase/* compile warnings

          People

            jplindst Jan Lindström (Inactive)
            jplindst Jan Lindström (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Git Integration

                Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.