Loading ...
Sorry, an error occurred while loading the content.

52569Regression introduced in patch 7.2.50: fwrite() failures incorrectly checked

Expand Messages
  • Dominique Pelle
    Nov 29, 2008
    • 0 Attachment
      Tests 58 and 59 fail when doing 'make test' with latest Vim-7.2.55
      (huge version, tested on Linux).

      I tried older versions:

      - Vim-7.2.49 .... All tests pass
      - Vim-7.2.50 .... Tests 58 & 59 fail.

      Patch 7.2.50 introduces the regression:

      --- 8< --- cut here --- 8< --- cut here --- 8< ---
      Patch 7.2.050
      Problem: Warnings for not checking return value of fwrite(). (Chip Campbell)
      Solution: Use the return value.
      Files: src/spell.c
      --- 8< --- cut here --- 8< --- cut here --- 8< ---


      Here is the mismatch in the test outcome:

      --- 8< --- cut here --- 8< --- cut here --- 8< ---
      $ diff -c test58.ok test58.failed
      *** test58.ok 2006-04-05 22:35:46.000000000 +0200
      --- test58.failed 2008-11-29 08:07:28.000000000 +0100
      ***************
      *** 40,48 ****
      gebletegek
      kepereneven
      everles gesvets etele
      ! kbltykk
      ! kprnfn
      ! *fls kswts tl
      elekwent
      elequint
      elekwint
      --- 40,48 ----
      gebletegek
      kepereneven
      everles gesvets etele
      ! gebletegek
      ! kepereneven
      ! everles gesvets etele
      elekwent
      elequint
      elekwint


      $ diff -c test59.ok test59.failed
      *** test59.ok 2006-04-05 22:39:13.000000000 +0200
      --- test59.failed 2008-11-29 08:07:29.000000000 +0100
      ***************
      *** 40,48 ****
      gebletegek
      kepereneven
      everles gesvets etele
      ! kbltykk
      ! kprnfn
      ! *fls kswts tl
      elekwent
      elequint
      elekwint
      --- 40,48 ----
      gebletegek
      kepereneven
      everles gesvets etele
      ! gebletegek
      ! kepereneven
      ! everles gesvets etele
      elekwent
      elequint
      --- 8< --- cut here --- 8< --- cut here --- 8< ---


      Looking at the official patch 7.2.50, I see that I/O errors are checked
      like this for example:

      int fwv = 1; /* collect return value of fwrite() to avoid
      warnings from picky compiler */

      ...snip...

      fwv &= fwrite(spin->si_sofofr, l, (size_t)1, fd); /* <sofofrom> */
      ...snip...
      fwv &= fwrite(spin->si_sofoto, l, (size_t)1, fd); /* <sofoto> */

      ...snip...

      if (fwv != 1)
      retval = FAIL;
      if (retval == FAIL)
      EMSG(_(e_write));


      This is an incorrect way of checking errors: fwrite() returns the number
      of items successfully written. So doing bit arithmetic on the return value
      can't possibly be correct.

      Correct would be to compare the return value of fwrite() with the number
      of items that is requested to be written:

      int error = 0;
      ...
      if (fwrite(spin->si_sofofr, l, (size_t)1, fd) != 1) /* <sofofrom> */
      error = 1;

      -- Dominique

      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_dev" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    • Show all 7 messages in this topic