[Python-checkins] r53110 - python/trunk/Lib/mailbox.py

[Neal Norwitz]

Is there any reason to use 2 rather than os.SEEK_END in this code?

> +        self._file.seek(0, 2)

n
--

On 12/20/06, andrew.kuchling <python-checkins at python.org> wrote:
> Author: andrew.kuchling
> Date: Wed Dec 20 20:48:20 2006
> New Revision: 53110
>
> Modified:
>    python/trunk/Lib/mailbox.py
> Log:
> [Apply length-checking.diff from bug #1599254]
>
> Add length checking to single-file mailbox formats: before doing a
> flush() on a mailbox, seek to the end and verify its length is
> unchanged, raising ExternalClashError if the file's length has
> changed.
>
> This fix avoids potential data loss if some other process appends to
> the mailbox file after the table of contents has been generated;
> instead of overwriting the modified file, you'll get the exception.
>
> I also noticed that the self._lookup() call in self.flush() wasn't
> necessary (everything that sets self._pending to True also calls
> self.lookup()), and replaced it by an assertion.
>
> 2.5 backport candidate.
>
>
> Modified: python/trunk/Lib/mailbox.py
> ==============================================================================
> --- python/trunk/Lib/mailbox.py (original)
> +++ python/trunk/Lib/mailbox.py Wed Dec 20 20:48:20 2006
> @@ -513,6 +513,7 @@
>          self._next_key = 0
>          self._pending = False   # No changes require rewriting the file.
>          self._locked = False
> +        self._file_length = None        # Used to record mailbox size
>
>      def add(self, message):
>          """Add message and return assigned key."""
> @@ -566,7 +567,21 @@
>          """Write any pending changes to disk."""
>          if not self._pending:
>              return
> -        self._lookup()
> +
> +        # In order to be writing anything out at all, self._toc must
> +        # already have been generated (and presumably has been modified
> +        # by adding or deleting an item).
> +        assert self._toc is not None
> +
> +        # Check length of self._file; if it's changed, some other process
> +        # has modified the mailbox since we scanned it.
> +        self._file.seek(0, 2)
> +        cur_len = self._file.tell()
> +        if cur_len != self._file_length:
> +            raise ExternalClashError('Size of mailbox file changed '
> +                                     '(expected %i, found %i)' %
> +                                     (self._file_length, cur_len))
> +
>          new_file = _create_temporary(self._path)
>          try:
>              new_toc = {}
> @@ -642,6 +657,7 @@
>          offsets = self._install_message(message)
>          self._post_message_hook(self._file)
>          self._file.flush()
> +        self._file_length = self._file.tell()  # Record current length of mailbox
>          return offsets
>
>
> @@ -733,6 +749,7 @@
>                  break
>          self._toc = dict(enumerate(zip(starts, stops)))
>          self._next_key = len(self._toc)
> +        self._file_length = self._file.tell()
>
>
>  class MMDF(_mboxMMDF):
> @@ -776,6 +793,8 @@
>                  break
>          self._toc = dict(enumerate(zip(starts, stops)))
>          self._next_key = len(self._toc)
> +        self._file.seek(0, 2)
> +        self._file_length = self._file.tell()
>
>
>  class MH(Mailbox):
> @@ -1201,7 +1220,9 @@
>          self._toc = dict(enumerate(zip(starts, stops)))
>          self._labels = dict(enumerate(label_lists))
>          self._next_key = len(self._toc)
> -
> +        self._file.seek(0, 2)
> +        self._file_length = self._file.tell()
> +
>      def _pre_mailbox_hook(self, f):
>          """Called before writing the mailbox to file f."""
>          f.write('BABYL OPTIONS:%sVersion: 5%sLabels:%s%s\037' %
> _______________________________________________
> Python-checkins mailing list
> Python-checkins at python.org
> http://mail.python.org/mailman/listinfo/python-checkins
>