[vim/vim] fix warning about possibly uninitialized eof var in fileio. (PR #12549)

Christian Brabandt

unread,

Jun 16, 2023, 3:33:25 AM6/16/23

to vim/vim, Subscribed

It is currently only initialized in the if (read_buffer) case but not in the else clause. There is the following code:

if (read_buffer)
  eof = FALSE;
else                  [1]
{
  if (filesize == 0)
   [...]
  else if (filesize > 0 ...  [4]
  {
      [...]
  if (curbuf->b_cryptstate->method_nr == CRYPT_M_SOD
				&& !eof && may_need_lseek)   [3]
  {
      [...]

  }
  }
  eof = size;        [2]

In the else condition [1], eof will only be initialized at [2], while it looks like it may be used at [3].

So technically, the eof variable could be accessed uninitialized in the else if case [4]. This can not happen, because filesize is initialized to zero, so Vim cannot run into this case in practice and after the first loop, eof will be initialized.

However, I think it's worth it, to move the initialization of the eof variable to the beginning of the function and don't init it in both branches of the if/else condition and also also do not depend on the implicit initialization of the filesize variable.

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

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

Commit Summary

8811ec4 fix warning about possibly uninitialized eof var in fileio.

File Changes

(1 file)

M src/fileio.c (3)

Patch Links:

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

codecov[bot]

unread,

Jun 16, 2023, 3:55:41 AM6/16/23

to vim/vim, Subscribed

Codecov Report

Merging #12549 (8811ec4) into master (46acad7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #12549   +/-   ##
=======================================
  Coverage   82.09%   82.10%           
=======================================
  Files         160      160           
  Lines      193625   193650   +25     
  Branches    43475    43481    +6     
=======================================
+ Hits       158966   158989   +23     
- Misses      21815    21816    +1     
- Partials    12844    12845    +1

Flag	Coverage Δ
huge-clang-none	`82.72% <100.00%> (-0.03%)`	⬇️
linux	`82.72% <100.00%> (-0.03%)`	⬇️
mingw-x64-HUGE	`76.60% <ø> (-0.01%)`	⬇️
mingw-x86-HUGE	`77.07% <ø> (+<0.01%)`	⬆️
windows	`78.20% <ø> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/fileio.c	`76.09% <100.00%> (ø)`

... and 20 files with indirect coverage changes

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Bram Moolenaar

unread,

Jun 16, 2023, 4:42:54 PM6/16/23

to vim/vim, Subscribed

Closed #12549 via 54f50cb.

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

You are receiving this because you are subscribed to this thread.

Bram Moolenaar

unread,

Jun 16, 2023, 5:03:45 PM6/16/23

to vim/vim, Subscribed

We might as well move the declaration there. The value is not used from
a previous loop, is it? The use at [3] is confusing though, it looks
like it wants the "eof" value from the previous loop. In that case
moving the declaration was a bad idea.

--
hundred-and-one symptoms of being an internet addict:
183. You move your coffeemaker next to your computer.

/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Bram Moolenaar

unread,

Jun 17, 2023, 9:59:36 AM6/17/23

to vim...@googlegroups.com, Christian Brabandt

My idea to move the declaration inside the loop does not appear to work
well. "make test_crypt" prompts for a key several times, and eventually
allocated memory is corrupted. Moving the declaration back to the start
of the function fixes most of this.

There still is one prompt for a key though. Hmm, I noticed before that
"eof" is used temporarily to store "size", which is confusing. When I
use a separate variable for that then it works better. Not sure why,
perhaps because "eof" is int and "size" is long?

--
hundred-and-one symptoms of being an internet addict:

187. You promise yourself that you'll only stay online for another
15 minutes...at least once every hour.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\

Reply all

Reply to author

Forward