# Python and Ruby implementations compared by the error density

Which programming language to start learning? Python or Ruby? Which one is better? Django or Ruby on Rails? Such questions can often be found on IT forums around the world\. I suggest comparing not the languages themselves, but their reference implementations: CPython and MRI\. In this article, we are going to cover the errors that were found by PVS\-Studio in these projects\. 

![0414_RubyCPython/image1.png](https://import.viva64.com/docx/blog/0414_RubyCPython/image1.png)

## Introduction

We took the latest versions of the source code from the repositories \([Ruby](https://github.com/ruby/ruby), Python\) for the analysis\. The project was scanned with PVS\-Studio v6\.06 static code analyzer\. Python can be easily compiled in Visual Studio; for Ruby you can use a Standalone version in the compilation monitoring mode\.

There weren't very many glaring errors: the majority of warnings are connected with the usage of macros, which get expanded into quite a suspicious code, from the point of view of the analyzer, but rather innocent from a developer's point of view\. We could start a long discussion on whether macros bring harm or good, but we can say for sure that the analyzer doesn't like them much\. To get rid of some annoying macro, there is an option to suppress false positives\. It's enough to write:

```cpp
//-V:RB_TYPE_P:501
```

And all the warnings by V501 diagnostic, where there is _RB\_TYPE\_P_ macro will disappear\. 

**Update\.** This article contains a few inaccuracies\. Please see the updated version here: "[Update on Analysis Results for CPython and Ruby](https://pvs-studio.com/en/blog/posts/cpp/0418/)"\.

## Python

![0414_RubyCPython/image2.png](https://import.viva64.com/docx/blog/0414_RubyCPython/image2.png)

### Fragment N1

```cpp
#ifdef MS_WINDOWS
typedef SOCKET SOCKET_T;
#else
typedef int SOCKET_T;
#endif
typedef struct {
  PyObject_HEAD
  SOCKET_T sock_fd; /* Socket file descriptor */
  ....
} PySocketSockObject;

static int
internal_select(PySocketSockObject *s,
                int writing,
                _PyTime_t interval,
                int connect)
{
  ....
  if (s->sock_fd < 0) // <=
    return 0;
  ....
}
```

[V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression 's\-\>sock\_fd < 0' is always false\. Unsigned type value is never < 0\. socketmodule\.c 655

The _SOCKET_ type in Windows is unsigned, so comparing it against null is meaningless\. The check of the _socket\(\)_ function returned a correct descriptor, it's necessary to compare its value with _INVALID\_SOCKET_\. It is worth noting that this comparison would work correctly in Linux, because there as the socket type, we have a signed type _int_ used, and the value \-1 indicates an error\. Nevertheless, it's better to use special macros or constants to check\. 

Several more similar checks that the analyzer issued warnings for\. 

* V547 Expression 's\-\>sock\_fd < 0' is always false\. Unsigned type value is never < 0\. \_ssl\.c 1702
* V547 Expression 'sock\-\>sock\_fd < 0' is always false\. Unsigned type value is never < 0\. \_ssl\.c 2018

### Fragment N2

```cpp
int ASN1_PRINTABLE_type(const unsigned char *s, int len)
{
  int c;
  int ia5 = 0;
  ....
  if (!(((c >= 'a') && (c <= 'z')) ||
        ((c >= 'A') && (c <= 'Z')) ||
        (c == ' ') ||                   // <=
        ((c >= '0') && (c <= '9')) ||
        (c == ' ') || (c == '\'') ||    // <=
        (c == '(') || (c == ')') ||
        (c == '+') || (c == ',') ||
        (c == '-') || (c == '.') ||
        (c == '/') || (c == ':') ||
        (c == '=') || (c == '?')))
    ia5 = 1;
  ....
}
```

[V501](https://pvs-studio.com/en/docs/warnings/v501/) There are identical sub\-expressions '\(c \=\= ' '\)' to the left and to the right of the '\|\|' operator\. a\_print\.c 77

A typical example of an error that occurred as a result of Copy\-Paste\. Quite often, when using large amounts of copied blocks, a programmer's attention falters and they forget to change a variable or a constant in them\. For example, in this case in a large conditional expression, the programmer confused the values that the variable _c_ is compared with\. We cannot say for sure, but it seems that the symbol of double quotation '"' was forgotten\. 

### Fragment N3

```cpp
static PyObject *
semlock_acquire(SemLockObject *self, PyObject *args, PyObject *kwds)
{
  ....
  HANDLE handles[2], sigint_event;
  ....
  /* prepare list of handles */
  nhandles = 0;
  handles[nhandles++] = self->handle;
  if (_PyOS_IsMainThread()) {
    sigint_event = _PyOS_SigintEvent();
    assert(sigint_event != NULL);
    handles[nhandles++] = sigint_event;
  }

  /* do the wait */
  Py_BEGIN_ALLOW_THREADS
  if (sigint_event != NULL) // <=
    ResetEvent(sigint_event);
  ....
}
```

[V614](https://pvs-studio.com/en/docs/warnings/v614/) Potentially uninitialized pointer 'sigint\_event' used\. semaphore\.c 120

In case the function _\_PyOS\_IsMainThread\(\)_ returns _false,_ the pointer _sigint\_event_ will remain uninitialized\. This will result in undefined behavior\. Such an error can easily be overlooked in the debug version, where a pointer is most likely to be initialized by a null\.

### Fragment N4

```cpp
#define BN_MASK2 (0xffffffffffffffffLL)
int BN_mask_bits(BIGNUM *a, int n)
{
  ....
  a->d[w] &= ~(BN_MASK2 << b); // <=
  ....
}
```

[V610](https://pvs-studio.com/en/docs/warnings/v610/) Undefined behavior\. Check the shift operator '<<'\. The left operand '\(0xffffffffffffffffLL\)' is negative\. bn\_lib\.c 796

Despite the fact that the code works in most cases, this expression is considered to be undefined behavior according to the standard\. You can find more details about the shifts of negative numbers in the article written by Andrey Karpov "[Wade not in unknown waters\. Part three](https://pvs-studio.com/en/blog/posts/cpp/0142/)"\. It's up to you to decide where it's necessary to avoid constructions whose results aren't guaranteed by the standard; but it's better not to do it at all; the analyzer agrees on that too\. 

```cpp
static PyObject *
binascii_b2a_qp_impl(PyModuleDef *module,
                     Py_buffer *data,
                     int quotetabs,
                     int istext,
                     int header)
{
  Py_ssize_t in, out;
  const unsigned char *databuf;
  ....
  if ((databuf[in] > 126) ||
      (databuf[in] == '=') ||
      (header && databuf[in] == '_') ||
      ((databuf[in] == '.') && (linelen == 0) &&
      (databuf[in+1] == '\n' || databuf[in+1] == '\r' ||
                                 databuf[in+1] == 0)) ||
      (!istext && ((databuf[in] == '\r') ||
                   (databuf[in] == '\n'))) ||
      ((databuf[in] == '\t' || databuf[in] == ' ') &&
           (in + 1 == datalen)) ||
      ((databuf[in] < 33) &&
       (databuf[in] != '\r') && (databuf[in] != '\n') &&
       (quotetabs ||
      (!quotetabs && ((databuf[in] != '\t') && // <=
             (databuf[in] != ' '))))))
  {
  ....
  }
  ....
}
```

[V728](https://pvs-studio.com/en/docs/warnings/v728/) An excessive check can be simplified\. The '\|\|' operator is surrounded by opposite expressions 'quotetabs' and '\!quotetabs'\. binascii\.c 1453

This fragment is not erroneous, nevertheless, we should take a closer look at it\. The warning is mostly a recommendation: the expression _'A \|\| \(\!A && B\)'_ can be simplified to _'A \|\| B'_: , which will make this quite complicated code easier to read\. 

Similar warnings:

* V728 An excessive check can be simplified\. The '\|\|' operator is surrounded by opposite expressions '\!type' and 'type'\. digest\.c 167
* V728 An excessive check can be simplified\. The '\|\|' operator is surrounded by opposite expressions '\!cipher' and 'cipher'\. evp\_enc\.c 120

### Fragment N5

```cpp
static int dh_cms_set_peerkey(....)
{
  ....
  int atype;
  ....
  /* Only absent parameters allowed in RFC XXXX */
  if (atype != V_ASN1_UNDEF && atype == V_ASN1_NULL)
    goto err;
   ....
}
```

[V590](https://pvs-studio.com/en/docs/warnings/v590/) Consider inspecting the 'atype \!\= \- 1 && atype \=\= 5' expression\. The expression is excessive or contains a misprint\. dh\_ameth\.c 670

It shouldn't seem odd, that errors in logical expressions occur even in large projects\. A logical expression is excessive here, and it can be simplified to '_atype \=\= V\_ASN1\_NULL_'\. Judging by the context, there is no error here, but such code looks really suspicious\. 

### Fragment N6

```cpp
static void cms_env_set_version(CMS_EnvelopedData *env)
{
  ....
  if (env->originatorInfo || env->unprotectedAttrs)
    env->version = 2;
  env->version = 0;
}
```

[V519](https://pvs-studio.com/en/docs/warnings/v519/) The 'env\-\>version' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 907, 908\. cms\_env\.c 908

It's hard to say what the author meant writing this code\. Perhaps _else_ is omitted here\. At this point there is no sense in _if,_ as the value of the '_env\-\>version'_ variable will be rewritten in any case\. 

```cpp
int
_PyState_AddModule(PyObject* module, struct PyModuleDef* def)
{
  PyInterpreterState *state;
  if (def->m_slots) {
    PyErr_SetString(PyExc_SystemError,
        "PyState_AddModule called on module with slots");
    return -1;
  }
  state = GET_INTERP_STATE();
  if (!def)
    return -1;
  ....
}
```

[V595](https://pvs-studio.com/en/docs/warnings/v595/) The 'self\-\>extra' pointer was utilized before it was verified against nullptr\. Check lines: 917, 923\. \_elementtree\.c 917

This is a traditional error, related to the null pointer dereferencing, which we find almost in every project\. Firstly, in the expression _'def\-\>m\_slots' _the programmer accessed by some address, and then it turned out that this address might have been null\. As a result the verification against _nullptr_ won't work, as we'll have the null pointer dereference, which will lead to undefined program behavior and to its crash, for instance\. 

## Ruby

![0414_RubyCPython/image3.png](https://import.viva64.com/docx/blog/0414_RubyCPython/image3.png)

### Fragment N1

```cpp
static void
vm_set_main_stack(rb_thread_t *th, const rb_iseq_t *iseq)
{
  VALUE toplevel_binding = rb_const_get(rb_cObject,
              rb_intern("TOPLEVEL_BINDING"));
  rb_binding_t *bind;
  rb_env_t *env;

  GetBindingPtr(toplevel_binding, bind);
  GetEnvPtr(bind->env, env);

  vm_set_eval_stack(th, iseq, 0, &env->block);

  /* save binding */
  if (bind && iseq->body->local_size > 0) {
    bind->env = vm_make_env_object(th, th->cfp);
  }
}
```

[V595](https://pvs-studio.com/en/docs/warnings/v595/) The 'bind' pointer was utilized before it was verified against nullptr\. Check lines: 377, 382\. vm\.c 377

A similar error was also encountered in the Ruby project\. The check _'if \(bind\)' _won't be of much help, because _bind_ was dereferenced a little earlier in the code\. There were more than 30 warnings of this kind, so there is no point in listing them all here\. 

### Fragment N2

```cpp
static int
code_page_i(....)
{
  table = realloc(table, count * sizeof(*table));
  if (!table) return ST_CONTINUE;
  ....
}
```

[V701](https://pvs-studio.com/en/docs/warnings/v701/) realloc\(\) possible leak: when realloc\(\) fails in allocating memory, original pointer 'table' is lost\. Consider assigning realloc\(\) to a temporary pointer\. file\.c 169

In this fragment we see that the value of _realloc _is saved in the same variable, which is used as an argument\. In case _realloc _returns _nullptr_, the initial pointer value will be lost, which will lead to a memory leak\. 

### Fragment N3

```cpp
static int
w32_symlink(UINT cp, const char *src, const char *link)
{
  ....
  BOOLEAN ret;

  typedef DWORD (WINAPI *create_symbolic_link_func)
                               (WCHAR*, WCHAR*, DWORD);
  static create_symbolic_link_func create_symbolic_link =
         (create_symbolic_link_func)-1;

  ....
  ret = create_symbolic_link(wlink, wsrc, flag);
  ALLOCV_END(buf);

  if (!ret) {
    int e = GetLastError();
    errno = map_errno(e);
    return -1;
  }
  return 0;
}
```

[V724](https://pvs-studio.com/en/docs/warnings/v724/) Converting type 'DWORD' to type 'BOOLEAN' can lead to a loss of high\-order bits\. Non\-zero value can become 'FALSE'\. win32\.c 4974

The _BOOLEAN_ type is used in the WinAPI as a logical type\. It is declared in the following way:

```cpp
typedef unsigned char BYTE;
typedef BYTE BOOLEAN;
```

DWORD is a 32\-bit unsigned number\. That's why if we cast DWORD 0xffffff00 value to BOOLEAN \(or any other, whose lowest bit is zero\), then it will become 0, i\.e\. FALSE\. 

### Fragment N4

```cpp
static VALUE
rb_str_split_m(int argc, VALUE *argv, VALUE str)
{
  ....
  char *ptr = RSTRING_PTR(str);
  long len = RSTRING_LEN(str);
  long start = beg;
  ....
  if (ptr+start == ptr+len)
    start++;
  ....
}
```

[V584](https://pvs-studio.com/en/docs/warnings/v584/) The 'ptr' value is present on both sides of the '\=\=' operator\. The expression is incorrect or it can be simplified\. string\.c 7211

In both parts of the comparison we have _ptr_ addition, therefore it can be removed:

```cpp
if (start == len)
```

But most likely, there is no error in this fragment\. However, we see quite often that programmers compare two different variables in such expressions\. That's why such comparisons are always worth reviewing\. 

## Overall results

Having analyzed all the warnings of the general analysis diagnostics, and removed all the false positives, I have come to the following conclusion concerning the error density:

![0414_RubyCPython/image4.png](https://import.viva64.com/docx/blog/0414_RubyCPython/image4.png)

The majority of warnings in Ruby were issued by the [V610](https://pvs-studio.com/en/docs/warnings/v610/) diagnostic \(369 warnings\!\), but even if we exclude them, the situation won't change much: Python takes the lead over Ruby in the number of suspicious fragments\. 

 The most frequent diagnostic turned out to be [V595](https://pvs-studio.com/en/docs/warnings/v595/) \- there were 17 warnings in Python, and 37 in Ruby\. 

Of course, it's much more interesting to look at the error density ratio\. Python also leaves Ruby well behind in this category\. Here are the results of the evaluations presented as a table: 

![0414_RubyCPython/image5.png](https://import.viva64.com/docx/blog/0414_RubyCPython/image5.png)

It may seem that the number of errors is quite large\. But it is not so\. Firstly, not all of the bugs are critical\. For example, the [V610](https://pvs-studio.com/en/docs/warnings/v610/) diagnostic that we have already mentioned, detects errors from the point of view of the C\+\+ language\. However, in practice for the set of compilers the result can always be correct\. Although these errors are still bugs, they don't affect the program in any way\. Secondly, we should take into account the size of the code\. That's why we can say that the quality of these projects is rather high\. At this point this may be rather subjective, because previously we haven't evaluated the error density of these projects\. We'll try to do that in the future, so that we can later compare the result of the checks\. 

## Conclusion

![0414_RubyCPython/image6.png](https://import.viva64.com/docx/blog/0414_RubyCPython/image6.png)

Python and Ruby are extremely popular: millions of developers use them to write code\. It's hard to find a large number of errors in a project when it is so actively used, regularly tested by another static analysis tool \(both projects get checked by Coverity\) and has community support\. Nonetheless, PVS\-Studio managed to find several suspicious fragments\. We should understand that these are regular checks which can make the life of programmers much easier\. The ideal, is to fix the error before the edits get to repository and release \- and a static analyzer can help best of all here\. 

I suggest running [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/) on your projects\.