Issues in source code

[Andey Karpov]

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

[Elena Stepanova (elenst)]

Assigning to Sergei so he could decide if there is anything to be done about it.

[Sergei Golubchik (sergii)]

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.