Bug #12611: Issues, reported by PVS-Studio static analyzer

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 2.1: WONTFIX, 2.2: DONE, 2.3: DONE

[pavel-belikov (Pavel Belikov)]

To demonstrate the capabilities of our analyzer, we regularly perform analysis of open source projects. We had recently checked the Ruby MRI project.

Here is the link to the article about it: http://www.viva64.com/en/b/0414/
Official page of the analyzer: http://www.viva64.com/en/pvs-studio/

If you have any questions, or if you are interested in the evaluation of our static analyzer or in any other source code quality control services that our company provides, please contact us at [e-mail].

[shyouhei (Shyouhei Urabe)]

• Fragment N1: GetBindingPtr() is a macro that assigns the argued variable.
• Fragment N2: we have our dedicated realloc() implementation that behaves differently than you assume (it bails out for errors).
• Fragment N3: CreateSymbolicLinkW() does return boolean, no data loss.
• Fragment N4: This is not an error like you commented, but I admit this is a bad habit.

Thank you for the report. I see no fatal errors. N4 is the only thing that I think worth fixing.

[mame (Yusuke Endoh)]

Shyouhei Urabe wrote:

• Fragment N1: GetBindingPtr() is a macro that assigns the argued variable.

It will cause no actual harm, but I think "if (bind && ...) {" seems pointless as OP said.

• Fragment N2: we have our dedicated realloc() implementation that behaves differently than you assume (it bails out for errors).

Really? It is realloc, not xrealloc. I cannot find a macro like "#define realloc ..." in win32/file.c.

• Fragment N3: CreateSymbolicLinkW() does return boolean, no data loss.

Then, I wonder why create_symbolic_link_func is declared so that it returns DWORD.

--
Yusuke Endoh [e-mail]

[nobu (Nobuyoshi Nakada)]

Yusuke Endoh wrote:

• Fragment N2: we have our dedicated realloc() implementation that behaves differently than you assume (it bails out for errors).

Really? It is realloc, not xrealloc. I cannot find a macro like "#define realloc ..." in win32/file.c.

Yes, it isn't xrealloc, but table is a copy of cp->table and the original pointer is kept even when realloc failed.

[shyouhei (Shyouhei Urabe)]

Nobuyoshi Nakada wrote:

Yusuke Endoh wrote:

• Fragment N2: we have our dedicated realloc() implementation that behaves differently than you assume (it bails out for errors).

Really? It is realloc, not xrealloc. I cannot find a macro like "#define realloc ..." in win32/file.c.

Yes, it isn't xrealloc, but table is a copy of cp->table and the original pointer is kept even when realloc failed.

Sorry. I was wrong here.

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

Applied in changeset r55729.

---

Fix Issues reported by PVS-Studio static analyzer

• vm.c (vm_set_main_stack): remove unnecessary check. toplevel
  binding must be initialized. [Bug #12611] (N1)
• win32/win32.c (w32_symlink): fix return type. [Bug #12611] (N3)
• string.c (rb_str_split_m): simplify the condition.
  Bug #12611

[usa (Usaku NAKAMURA)]

• Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN to 2.1: WONTFIX, 2.2: REQUIRED, 2.3: REQUIRED

[usa (Usaku NAKAMURA)]

• Backport changed from 2.1: WONTFIX, 2.2: REQUIRED, 2.3: REQUIRED to 2.1: WONTFIX, 2.2: DONE, 2.3: REQUIRED

ruby_2_2 r55925 merged revision(s) 55729.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.1: WONTFIX, 2.2: DONE, 2.3: REQUIRED to 2.1: WONTFIX, 2.2: DONE, 2.3: DONE

ruby_2_3 r55959 merged revision(s) 55729.