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

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

Expand Messages
  • Bram Moolenaar
    Nov 29, 2008
      Dominique Pelle wrote:

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

      Strange, for me all tests pass.

      > 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;

      All the fwrite() calls have a "number of elements" argument of 1. When
      fwrite succeeds it returns 1, when it fails it returns 0. So and-ing
      all the 1 values together gives 1. If there is one zero the and-ed
      value is zero.

      Perhaps frwrite() actually returns something different for you? Or
      there is a problem with the conversion of size_t to int? You can change
      the type of "fwv" to size_t and see if that makes a difference.

      --
      ** Hello and Welcome to the Psychiatric Hotline **
      If you are obsessive-compulsive, please press 1 repeatedly.
      If you are co-dependent, please ask someone to press 2.
      If you have multiple personalities, please press 3, 4, 5 and 6.
      If you are paranoid-delusional, we know who you are and what you want
      - just stay on the line so we can trace the call.
      If you are schizophrenic, listen carefully and a little voice will
      tell you which number to press next.
      If you are manic-depressive, it doesn't matter which number you press
      - no one will answer.
      If you suffer from panic attacks, push every button you can find.
      If you are sane, please hold on - we have the rest of humanity on the
      other line and they desparately want to ask you a few questions.

      /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
      /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
      \\\ download, build and distribute -- http://www.A-A-P.org ///
      \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

      --~--~---------~--~----~------------~-------~--~----~
      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