Patchwork [1,of,2] verify: always check rawsize

login
register
mail settings
Submitter Jun Wu
Date May 11, 2017, 11:13 p.m.
Message ID <1008583138d3dca114bc.1494544410@x1c>
Download mbox | patch
Permalink /patch/20581/
State Accepted
Headers show

Comments

Jun Wu - May 11, 2017, 11:13 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1494539522 25200
#      Thu May 11 14:52:02 2017 -0700
# Node ID 1008583138d3dca114bc9d4998a27e845fbc13b0
# Parent  1ada3d18e7fbc9069910f2c036992d2f2b28e058
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 1008583138d3
verify: always check rawsize

Previously, verify only checks "rawsize == len(rawtext)", if
"len(fl.read()) != fl.size()".

With flag processor, "len(fl.read()) != fl.size()" does not necessarily mean
"rawsize == len(rawtext)". So we may miss a useful check.

This patch removes the "if len(fl.read()) != fl.size()" condition so the
rawsize check is always performed.

With the condition removed, "fl.read(n)" looks unnecessary so a comment was
added to explain the side effect is wanted.
Augie Fackler - May 13, 2017, 2:13 a.m.
On Thu, May 11, 2017 at 04:13:30PM -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1494539522 25200
> #      Thu May 11 14:52:02 2017 -0700
> # Node ID 1008583138d3dca114bc9d4998a27e845fbc13b0
> # Parent  1ada3d18e7fbc9069910f2c036992d2f2b28e058
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 1008583138d3
> verify: always check rawsize

This one is queued, but I've got some qualms about patch 2.

>
> Previously, verify only checks "rawsize == len(rawtext)", if
> "len(fl.read()) != fl.size()".
>
> With flag processor, "len(fl.read()) != fl.size()" does not necessarily mean
> "rawsize == len(rawtext)". So we may miss a useful check.
>
> This patch removes the "if len(fl.read()) != fl.size()" condition so the
> rawsize check is always performed.
>
> With the condition removed, "fl.read(n)" looks unnecessary so a comment was
> added to explain the side effect is wanted.
>
> diff --git a/mercurial/verify.py b/mercurial/verify.py
> --- a/mercurial/verify.py
> +++ b/mercurial/verify.py
> @@ -428,11 +428,12 @@ class verifier(object):
>                  #     use either "text" (external), or "rawtext" (in revlog).
>                  try:
> -                    l = len(fl.read(n))
> +                    fl.read(n) # side effect: read content and do checkhash
>                      rp = fl.renamed(n)
> -                    if l != fl.size(i):
> -                        # the "L1 == L2" check
> -                        if len(fl.revision(n, raw=True)) != fl.rawsize(i):
> -                            self.err(lr, _("unpacked size is %s, %s expected") %
> -                                     (l, fl.size(i)), f)
> +                    # the "L1 == L2" check
> +                    l1 = fl.rawsize(i)
> +                    l2 = len(fl.revision(n, raw=True))
> +                    if l1 != l2:
> +                        self.err(lr, _("unpacked size is %s, %s expected") %
> +                                 (l2, l1), f)
>                  except error.CensoredNodeError:
>                      # experimental config: censor.policy
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/verify.py b/mercurial/verify.py
--- a/mercurial/verify.py
+++ b/mercurial/verify.py
@@ -428,11 +428,12 @@  class verifier(object):
                 #     use either "text" (external), or "rawtext" (in revlog).
                 try:
-                    l = len(fl.read(n))
+                    fl.read(n) # side effect: read content and do checkhash
                     rp = fl.renamed(n)
-                    if l != fl.size(i):
-                        # the "L1 == L2" check
-                        if len(fl.revision(n, raw=True)) != fl.rawsize(i):
-                            self.err(lr, _("unpacked size is %s, %s expected") %
-                                     (l, fl.size(i)), f)
+                    # the "L1 == L2" check
+                    l1 = fl.rawsize(i)
+                    l2 = len(fl.revision(n, raw=True))
+                    if l1 != l2:
+                        self.err(lr, _("unpacked size is %s, %s expected") %
+                                 (l2, l1), f)
                 except error.CensoredNodeError:
                     # experimental config: censor.policy