Gregory, I agree, but in this particular case I chose the “deepest” call which 
insert flags into the revlog such that I cover all the possible call paths. 
Although to be fair, another good alternative would be revlog._addrevision. I’m 
happy to move the operation there, if you guys find it more appropriate.

From: Gregory Szorc <gregory.sz...@gmail.com>
Date: Friday, November 25, 2016 at 5:27 PM
To: Cotizo Sima <cot...@fb.com>
Cc: mercurial-devel <mercurial-devel@mercurial-scm.org>
Subject: Re: [PATCH] revlog: ensure that flags do not overflow 2 bytes

On Fri, Nov 25, 2016 at 5:24 AM, Cotizo Sima 
<cot...@fb.com<mailto:cot...@fb.com>> wrote:
# HG changeset patch
# User Cotizo Sima <cot...@fb.com<mailto:cot...@fb.com>>
# Date 1480079985 28800
#      Fri Nov 25 05:19:45 2016 -0800
# Node ID 4b311b730614941db6409f560ab9451bec74be75
# Parent  a3163433647108b7bec8fc45896db1c20b18ab21
revlog: ensure that flags do not overflow 2 bytes

This patch adds a line that ensures we are not setting by mistake a set of flags
overlfowing the 2 bytes they are allocated. Given the way the data is packed in
the revlog header, overflowing 2 bytes will result in setting a wrong offset.

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -72,6 +72,7 @@
     return int(q & 0xFFFF)

 def offset_type(offset, type):
+    type &= 0xFFFF
     return long(long(offset) << 16 | type)

 _nullhash = hashlib.sha1(nullid)

I'm a believer in failing fast. If this new code comes into play, we've already 
lost to a bug. I think instead we should raise ValueError if type > 65535.

_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to