[PATCH stable] convert.cvs: Initialize state variable

[Mads Kiilerich]

On 03/24/2010 08:48 AM, Patrick Mézard wrote:
> Le 24/03/10 01:54, Mads Kiilerich a écrit :
>> # HG changeset patch
>> # User Mads Kiilerich<mads at kiilerich.com>
>> # Date 1269392066 -3600
>> # Branch stable
>> # Node ID e8230de3efb4a8b834b93c01c60785f9f55987a9
>> # Parent  f621cddc06594a5c4e67ae910fd1ed9682a571a4
>> convert.cvs: Initialize state variable
>>
>> Avoids
>> UnboundLocalError: local variable 'mode' referenced before assignment
>> when cvs fails
>>
>> diff --git a/hgext/convert/cvs.py b/hgext/convert/cvs.py
>> --- a/hgext/convert/cvs.py
>> +++ b/hgext/convert/cvs.py
>> @@ -227,6 +227,7 @@
>>           self.writep.flush()
>>
>>           data = ""
>> +        mode = ""
>>           while 1:
>>               line = self.readp.readline()
>>               if line.startswith("Created ") or line.startswith("Updated "):
>
> -1 on this fix: http://mercurial.selenic.com/bts/msg9060

Ok. But I am -2 on having code paths where external input (no matter how 
unexpected and wrong) can cause use of uninitialized variables and crashes.

Why should data be initialized when mode isn't?

Initializing mode might not make the code correct, but it removes one 
problem.

How about something like

diff --git a/hgext/convert/cvs.py b/hgext/convert/cvs.py
--- a/hgext/convert/cvs.py
+++ b/hgext/convert/cvs.py
@@ -226,7 +226,8 @@
          self.writep.write("Directory .\n%s\nco\n" % self.realroot)
          self.writep.flush()

-        data = ""
+        data = None
+        mode = ""
          while 1:
              line = self.readp.readline()
              if line.startswith("Created ") or line.startswith("Updated "):
@@ -244,6 +245,8 @@
                  data = chunkedread(self.readp, count)
              else:
                  if line == "ok\n":
+                    if data is None:
+                        raise util.Abort(_('CVS response "ok" without 
data'))
                      return (data, "x" in mode and "x" or "")
                  elif line.startswith("E "):
                      self.ui.warn(_("cvs server: %s\n") % line[2:])


/Mads