[vim/vim] session.c is somewhat hard to read due to explicit error checking (Issue #20819)

3 views

Skip to first unread message

Illia Bobyr

unread,

Jul 22, 2026, 7:36:47 PM (2 days ago) Jul 22

to vim/vim, Subscribed

illia-bobyr created an issue (vim/vim#20819)

tl;dr:

I think session.c can be refactored to make the "happy path" more obvious and to unify error handling patterns.
I have this refactoring here: refactor-session-c-to-hide-error-checking

I would like to post it for review.
It is probably easier to review it one step at a time.
So, here is the first commit from this branch: #20818

As all the changes ultimately serve the same goal, this issue groups them.
As well as serves as a single point for discussion, if necessary.

Long version:

While making changes in session.c I found that it is sometimes difficult to follow the logic due to constant explicit error checks.
session.c writes to external files in small chunks, and every output operation result is checked.
While the error checking is great, it is also an unlikely execution path.
Yet, in the source code, it is mixed with the very common "happy path" on an equal footing.
Here are a few examples:

            if (fputs(":$argadd ", fd) < 0
                    || ses_put_fname(fd, s, flagp) == FAIL
                    || put_eol(fd) == FAIL)
            {
                vim_free(buf);
                return FAIL;
            }

    if (count > 0 && (fprintf(fd, fr->fr_layout == FR_COL
                    ? ":%dwincmd k" : ":%dwincmd h", count) < 0
                || put_eol(fd) == FAIL))

While it is not impossible to understand what each expression does, it is harder than an equivalent code that invokes output operations in sequence.
And running functions with side effects in the if expressions, I think, is somewhat uncommon.
As such, it impedes habits of reading the "common" code that most people acquire over time.
When it is harder to read the code, it is more likely that it would not be read as carefully or would be skipped to avoid extra mental burden.

As all the error checking was repeated in every function, there are actually 3 different patterns used to handle errors:

Return immediately on any error.
Jump to a cleanup block on error.
Set a flag on any error, and check this flag on all subsequent operations and at the end of the function.

I propose a few macros to hide error checking and a small change to the helper functions API.
Using 3 different approaches to error handling hurts code consistency. So the macros use the second jump to a cleanup block.

I've refactored all the functions. But refactoring of ex_mkrc() is not finished.
I've extracted some logic from it into a helper to make it easier to follow, but it is still very complex.
It handles so many cases that, in my mind, it certainly deserves further simplification.
Yet, because it is so difficult to follow all the logic paths at the same time, I've stopped for now.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.

h_east

unread,

Jul 23, 2026, 1:44:12 PM (20 hours ago) Jul 23

to vim/vim, Subscribed

h-east left a comment (vim/vim#20819)

Replied on #20818. In short, I would rather not move session.c to this style; the reasoning is there.