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

Build failure on MacOS on compiling the file btr/btr0btr.cc

Details

    Description

      Attempt to build MariaDB server on mac OS leads to compilation error:

      [ 57%] Building CXX object storage/innobase/CMakeFiles/innobase.dir/btr/btr0btr.cc.o
      In file included from /Users/shulga/projects/mariadb/server-10.5/storage/innobase/btr/btr0btr.cc:41:
      In file included from /Users/shulga/projects/mariadb/server-10.5/storage/innobase/include/trx0trx.h:34:
      In file included from /Users/shulga/projects/mariadb/server-10.5/storage/innobase/include/trx0xa.h:27:
      In file included from /Users/shulga/projects/mariadb/server-10.5/sql/handler.h:34:
      /Users/shulga/projects/mariadb/server-10.5/sql/structs.h:877:14: error: implicit conversion loses integer
            precision: 'ulong' (aka 'unsigned long') to '__darwin_suseconds_t' (aka 'int')
            [-Werror,-Wshorten-64-to-32]
          tv_usec= usec;
                 ~ ^~~~
      1 error generated.
      

      Attachments

        Activity

          shulga Dmitry Shulga added a comment -

          The patch is ready for review in the branch bb-10.5-MDEV-24115

          shulga Dmitry Shulga added a comment - The patch is ready for review in the branch bb-10.5- MDEV-24115

          ifdefs aren't particularly good for maintainability and readability. Why not to cast unconditionally? Or to use uint for the constructor argument?

          serg Sergei Golubchik added a comment - ifdefs aren't particularly good for maintainability and readability. Why not to cast unconditionally? Or to use uint for the constructor argument?
          shulga Dmitry Shulga added a comment -

          Timeval is widely used across the source code and some of methods (e.g. Field::store_timestamp)
          call Timeval constructor passing its arguments directly to constructor's arguments that results in
          implicit dependency of Timeval constructor's arguments on type of calling method's arguments (Field::store_timestamp in our case).

          Changing types of argument in every method that calls Timeval constructor requires more changes than just one cast inside constructor of Timeval
          So, from my point of view it is better to cast unconditionally to uint inside Tmeval constructor.

          shulga Dmitry Shulga added a comment - Timeval is widely used across the source code and some of methods (e.g. Field::store_timestamp) call Timeval constructor passing its arguments directly to constructor's arguments that results in implicit dependency of Timeval constructor's arguments on type of calling method's arguments (Field::store_timestamp in our case). Changing types of argument in every method that calls Timeval constructor requires more changes than just one cast inside constructor of Timeval So, from my point of view it is better to cast unconditionally to uint inside Tmeval constructor.

          ok. by "cast unconditionally" I meant

          -    tv_usec= usec;
          +    tv_usec= (suseconds_t)usec;
          

          although uint might work too

          serg Sergei Golubchik added a comment - ok. by "cast unconditionally" I meant - tv_usec= usec; + tv_usec= (suseconds_t)usec; although uint might work too
          shulga Dmitry Shulga added a comment -

          The type suseconds_t conforms to POSIX.1-2001 and later.
          Release edition POSIX.1-2001 confused me a bit and therefore I proposed casting to uint.
          Now I rechecked manual pages and found that timeval is also declared conforming to POSIX.1-2001 and later.
          Since timeval struct is widely and successfully used both in MariaDB and MySQL I believe that we can
          suppose that every environment where MariaDB can be used should conform to POSIX.1-2001.
          So, I agree with your remark and will change casting type from uint to suseconds_t.

          shulga Dmitry Shulga added a comment - The type suseconds_t conforms to POSIX.1-2001 and later. Release edition POSIX.1-2001 confused me a bit and therefore I proposed casting to uint. Now I rechecked manual pages and found that timeval is also declared conforming to POSIX.1-2001 and later. Since timeval struct is widely and successfully used both in MariaDB and MySQL I believe that we can suppose that every environment where MariaDB can be used should conform to POSIX.1-2001. So, I agree with your remark and will change casting type from uint to suseconds_t.

          People

            shulga Dmitry Shulga
            shulga Dmitry Shulga
            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.