Details

    • Type: Bug
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:

      Description

      I checked the MariaDB project using the PVS-Studio static code analyzer.
      I just glanced through the code but managed to find a few obviously odd
      fragments. Below I will cite the analyzer-generated messages I have
      studied and the corresponding code fragments. I hope this will help to
      improve the project a bit.

      You may review other odd fragments by downloading PVS-Studio from here:
      http://www.viva64.com/en/pvs-studio-download/

      I can also give you a registration key for some time.
      You are welcome to ask questions here:
      http://www.viva64.com/en/about-feedback/

      -----------
      V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. libevent win32.c 427

      struct win32op {
      int fd_setsz;
      struct win_fd_set *readset_in;
      struct win_fd_set *writeset_in;
      struct win_fd_set *readset_out;
      struct win_fd_set *writeset_out;
      struct win_fd_set *exset_out;
      RB_HEAD(event_map, event_entry) event_root;
      };

      void
      win32_dealloc(struct event_base *_base, void *arg)
      {
      struct win32op *win32op = arg;
      ...
      memset(win32op, 0, sizeof(win32op));
      free(win32op);
      }
      -----------
      V547 Expression 'str [0] != 'a' || str [0] != 'A'' is always true. Probably the '&&' operator should be used here. mysqlclient my_time.c 348

      enum enum_mysql_timestamp_type
      str_to_datetime(...)
      {
      ...
      else if (str[0] != 'a' || str[0] != 'A')
      ...
      }
      -----------
      V501 There are identical sub-expressions to the left and to the right of the '!=' operator. sql opt_subselect.cc 598

      static
      bool subquery_types_allow_materialization(Item_in_subselect *in_subs)
      {
      ...
      case TIME_RESULT:
      if (mysql_type_to_time_type(outer->field_type()) !=
      mysql_type_to_time_type(outer->field_type()))
      DBUG_RETURN(FALSE);
      ...
      }
      -----------
      V547 Expression 'thd->net.vio->sd >= 0' is always true. Unsigned type value is always >= 0. sql scheduler.cc 515

      typedef UINT_PTR SOCKET;

      #define my_socket SOCKET

      struct st_vio
      {
      my_socket sd;
      ...
      };

      static void libevent_connection_close(THD *thd)
      {
      ...
      if (thd->net.vio->sd >= 0) // not already closed

      { end_connection(thd); close_connection(thd, 0, 1); }

      ...
      }
      -----------
      V501 There are identical sub-expressions 'wcsicmp (file_part, L"mysqld.exe") != 0' to the left and to the right of the '&&' operator. winservice winservice.c 118

      int get_mysql_service_properties(...)
      {
      ...
      if(wcsicmp(file_part, L"mysqld.exe") != 0 &&
      wcsicmp(file_part, L"mysqld.exe") != 0 &&
      wcsicmp(file_part, L"mysqld-nt.exe") != 0)

      { /* The service executable is not mysqld. */ goto end; }

      ...
      }
      -----------
      V519 The 'errmsg' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 222, 224. sql derror.cc 224

      static bool read_texts(...)
      {
      ...
      case 2:
      errmsg= "Incompatible header in messagefile '%s'. Probably from another version of MariaDB";
      case 1:
      errmsg= "Can't read from messagefile '%s'";
      break;
      ...
      }

      Need break operator.
      -----------
      V525 The code containing the collection of similar blocks. Check items '0', '1', '2', '3', '4', '1', '6' in lines 618, 620, 622, 627, 629, 631, 633. sql records.cc 618

      static int rr_cmp(uchar *a,uchar *b)
      {
      if (a[0] != b[0])
      return (int) a[0] - (int) b[0];
      if (a[1] != b[1])
      return (int) a[1] - (int) b[1];
      if (a[2] != b[2])
      return (int) a[2] - (int) b[2];
      #if MAX_REFLENGTH == 4
      return (int) a[3] - (int) b[3];
      #else
      if (a[3] != b[3])
      return (int) a[3] - (int) b[3];
      if (a[4] != b[4])
      return (int) a[4] - (int) b[4];
      if (a[5] != b[5])
      return (int) a[1] - (int) b[5];
      if (a[6] != b[6])
      return (int) a[6] - (int) b[6];
      return (int) a[7] - (int) b[7];
      #endif
      }

      Check:
      return (int) a[1] - (int) b[5];

      Andrey Karpov, MVP
      Cand. Sc. (Physics and Mathematics), CTO
      Program Verification Systems Co., Ltd.
      URL: www.viva64.com
      E-Mail: karpov[@]viva64.com

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            elenst Elena Stepanova added a comment -

            Re: Issues in source code
            Assigning to Sergei so he could decide if there is anything to be done about it.

            Show
            elenst Elena Stepanova added a comment - Re: Issues in source code Assigning to Sergei so he could decide if there is anything to be done about it.
            Hide
            serg Sergei Golubchik added a comment -

            Re: Issues in source code
            Thanks for checking MariaDB sources!

            All fixed [almost].

            Now, if you're interested, here's a reply to every found problem, one by one:

            1. wrong memset in libevent.

            not fixed. the line is obviously wrong, but that memory is freed on the next line, so memset is not needed at all. So, as libevent is a third-party library and we'd prefer not to deviate from the upstream, I didn't correct this issue.

            2. (in my_time.c) Already fixed. About two months ago I've looked at what you've found in MySQL code (as listed in the article on viva64.com) and fixed all that was applicable.

            3. (in opt_subselect.cc) Fixed.

            4. (in scheduler.cc) Fixed.

            5. (in winservice.c) Fixed.

            6. (in derror.c) Already fixed.

            7. (in records.cc) Already fixed. It was in your MySQL defect list.

            Show
            serg Sergei Golubchik added a comment - Re: Issues in source code Thanks for checking MariaDB sources! All fixed [almost] . Now, if you're interested, here's a reply to every found problem, one by one: 1. wrong memset in libevent. not fixed. the line is obviously wrong, but that memory is freed on the next line, so memset is not needed at all. So, as libevent is a third-party library and we'd prefer not to deviate from the upstream, I didn't correct this issue. 2. (in my_time.c) Already fixed. About two months ago I've looked at what you've found in MySQL code (as listed in the article on viva64.com) and fixed all that was applicable. 3. (in opt_subselect.cc) Fixed. 4. (in scheduler.cc) Fixed. 5. (in winservice.c) Fixed. 6. (in derror.c) Already fixed. 7. (in records.cc) Already fixed. It was in your MySQL defect list.
            Hide
            ratzpo Rasmus Johansson added a comment -

            Launchpad bug id: 893522

            Show
            ratzpo Rasmus Johansson added a comment - Launchpad bug id: 893522

              People

              • Assignee:
                serg Sergei Golubchik
                Reporter:
                andeykarpov Andey Karpov
              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: