Project

General

Profile

Actions

Bug #12611

closed

Issues, reported by PVS-Studio static analyzer

Added by pavel-belikov (Pavel Belikov) over 7 years ago. Updated over 7 years ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:76519]

Description

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 .

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

  • 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.

Updated by mame (Yusuke Endoh) over 7 years ago

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

Updated by nobu (Nobuyoshi Nakada) over 7 years ago

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.

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

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.

Actions #5

Updated by nobu (Nobuyoshi Nakada) over 7 years ago

  • 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

Updated by usa (Usaku NAKAMURA) over 7 years ago

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

Updated by usa (Usaku NAKAMURA) over 7 years ago

  • 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.

Updated by nagachika (Tomoyuki Chikanaga) over 7 years ago

  • 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.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0