[vim/vim] Fix a Coverity warning, add a test and reduce the calls to clear_tv() (PR #14845)

9 views
Skip to first unread message

Yegappan Lakshmanan

unread,
May 24, 2024, 10:12:09 PMMay 24
to vim/vim, Subscribed

You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/14845

Commit Summary

  • 7f4f1a0 Fix a coverity warning, add a test and reduce the calls to clear_tv()

File Changes

(2 files)

Patch Links:

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/14845@github.com>

Yegappan Lakshmanan

unread,
May 24, 2024, 10:30:13 PMMay 24
to vim/vim, Push

@yegappan pushed 1 commit.

—
View it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/14845/push/18581140791@github.com>

Christian Brabandt

unread,
May 25, 2024, 4:01:14 AMMay 25
to vim/vim, Subscribed

@chrisbra commented on this pull request.

In src/eval.c:

>      clear_tv(&var1);
-    return p;
+    clear_tv(&var2);
+    return rc == OK ? p : NULL;

This got me wondering. Is it fine to call clear_tv() for var1 and var2 twice? Once on line 1890+1891 and then here on line 1897 and 1898 again when the loop finished?

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/14845/review/2078825660@github.com>

Yegappan Lakshmanan

unread,
May 25, 2024, 8:09:54 AMMay 25
to vim/vim, Subscribed

@yegappan commented on this pull request.

In src/eval.c:

>      clear_tv(&var1);
-    return p;
+    clear_tv(&var2);
+    return rc == OK ? p : NULL;

Only simple types like number, string, bool and float can be used to index a List, Dict, Blob, Object or a Class.
Other composite types are not supported as index values. With the simple types, clear_tv() can be called twice
without causing any reference counting issues. When encountering an unsupported composite type, this
function will jump to the end of the function, where the type is cleared only once.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/14845/review/2078957237@github.com>

Yegappan Lakshmanan

unread,
May 25, 2024, 8:30:11 AMMay 25
to vim/vim, Subscribed

@yegappan commented on this pull request.

In src/eval.c:

>      clear_tv(&var1);
-    return p;
+    clear_tv(&var2);
+    return rc == OK ? p : NULL;

Patch 8.0.0352 (f06e5a5) is related to this.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/14845/review/2078960300@github.com>

Yegappan Lakshmanan

unread,
May 25, 2024, 9:12:34 AMMay 25
to vim/vim, Push

@yegappan pushed 1 commit.

  • dc12cf5 After clearing the type, set it to VAR_UNKNOWN

—
View it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/14845/push/18584479950@github.com>

Christian Brabandt

unread,
May 25, 2024, 2:24:03 PMMay 25
to vim/vim, Subscribed

@chrisbra commented on this pull request.

In src/eval.c:

>      clear_tv(&var1);
-    return p;
+    clear_tv(&var2);
+    return rc == OK ? p : NULL;

okay thanks

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/14845/review/2079100109@github.com>

Christian Brabandt

unread,
May 25, 2024, 2:26:38 PMMay 25
to vim/vim, Subscribed

Closed #14845 via dbac0da.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/14845/issue_event/12933665799@github.com>

Reply all
Reply to author
Forward
0 new messages