Patchwork [bitbake-devel] cooker: remove some redundant file manipulation code

login
register
mail settings
Submitter Stanacar, StefanX
Date Nov. 12, 2013, 2:07 p.m.
Message ID <1384265249-11307-1-git-send-email-stefanx.stanacar@intel.com>
Download mbox | patch
Permalink /patch/61505/
State New
Headers show

Comments

Stanacar, StefanX - Nov. 12, 2013, 2:07 p.m.
There is no need to call close() for files opened in "with" statements.
Also, in some places files content was unnecessarily copied from a list to a string,
and there is no reason to do that.

Signed-off-by: Stefan Stanacar <stefanx.stanacar@intel.com>
---
 bitbake/lib/bb/cooker.py | 34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)
Chris Larson - Nov. 12, 2013, 3:17 p.m.
On Tue, Nov 12, 2013 at 7:07 AM, Stefan Stanacar <stefanx.stanacar@intel.com
> wrote:

> +        total = ''
>          with open(default_file, 'r') as f:
> -            contents = f.readlines()
> -        f.close()
> -
> -        total = ""
> -        for c in contents:
> -            total += c
> +            for line in f:
> +                total += line
>
>          total += "#added by hob"
>          total += "\n%s += \"%s\"\n" % (var, val)
>
>          with open(default_file, 'w') as f:
>              f.write(total)
> -        f.close()
>

This is weird. Have they ever heard of opening a file in append mode? :)
And obviously, splitting the contents into lines, then re-concatenating
those lines into a string blob makes no sense either.
Cristiana Voicu - Nov. 12, 2013, 3:44 p.m.
It’s my mistake. I remember I sent more similar methods, some of them modifying the contents. Hope this is a good excuse ?
I will send a patch that uses append.
Cristiana
From: bitbake-devel-bounces@lists.openembedded.org [mailto:bitbake-devel-bounces@lists.openembedded.org] On Behalf Of Chris Larson

Sent: Tuesday, November 12, 2013 5:18 PM
To: Stanacar, StefanX
Cc: bitbake-devel@lists.openembedded.org
Subject: Re: [bitbake-devel] [PATCH] cooker: remove some redundant file manipulation code


On Tue, Nov 12, 2013 at 7:07 AM, Stefan Stanacar <stefanx.stanacar@intel.com<mailto:stefanx.stanacar@intel.com>> wrote:
+        total = ''
         with open(default_file, 'r') as f:
-            contents = f.readlines()
-        f.close()
-
-        total = ""
-        for c in contents:
-            total += c
+            for line in f:
+                total += line

         total += "#added by hob"
         total += "\n%s += \"%s\"\n" % (var, val)

         with open(default_file, 'w') as f:
             f.write(total)
-        f.close()

This is weird. Have they ever heard of opening a file in append mode? :) And obviously, splitting the contents into lines, then re-concatenating those lines into a string blob makes no sense either.
--
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

Patch

diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py
index 0af4558..3aa69ed 100644
--- a/bitbake/lib/bb/cooker.py
+++ b/bitbake/lib/bb/cooker.py
@@ -202,20 +202,16 @@  class BBCooker:
         #add append var operation to the end of default_file
         default_file = bb.cookerdata.findConfigFile(default_file, self.data)
 
+        total = ''
         with open(default_file, 'r') as f:
-            contents = f.readlines()
-        f.close()
-
-        total = ""
-        for c in contents:
-            total += c
+            for line in f:
+                total += line
 
         total += "#added by hob"
         total += "\n%s += \"%s\"\n" % (var, val)
 
         with open(default_file, 'w') as f:
             f.write(total)
-        f.close()
 
         #add to history
         loginfo = {"op":append, "file":default_file, "line":total.count("\n")}
@@ -244,7 +240,6 @@  class BBCooker:
             if topdir in conf_file:
                 with open(conf_file, 'r') as f:
                     contents = f.readlines()
-                f.close()
 
                 lines = self.data.varhistory.get_variable_lines(var, conf_file)
                 for line in lines:
@@ -270,12 +265,8 @@  class BBCooker:
                         for ii in range(begin_line, end_line):
                             contents[ii] = "#" + contents[ii]
 
-                total = ""
-                for c in contents:
-                    total += c
                 with open(conf_file, 'w') as f:
-                    f.write(total)
-                f.close()
+                    f.writelines(contents)
 
         if replaced == False:
             #remove var from history
@@ -284,13 +275,10 @@  class BBCooker:
             #add var to the end of default_file
             default_file = bb.cookerdata.findConfigFile(default_file, self.data)
 
-            with open(default_file, 'r') as f:
-                contents = f.readlines()
-            f.close()
-
             total = ""
-            for c in contents:
-                total += c
+            with open(default_file, 'r') as f:
+                for line in f:
+                    total += line
 
             #add the variable on a single line, to be easy to replace the second time
             total += "\n#added by hob"
@@ -298,7 +286,6 @@  class BBCooker:
 
             with open(default_file, 'w') as f:
                 f.write(total)
-            f.close()
 
             #add to history
             loginfo = {"op":set, "file":default_file, "line":total.count("\n")}
@@ -312,7 +299,6 @@  class BBCooker:
             if topdir in conf_file:
                 with open(conf_file, 'r') as f:
                     contents = f.readlines()
-                f.close()
 
                 lines = self.data.varhistory.get_variable_lines(var, conf_file)
                 for line in lines:
@@ -335,12 +321,8 @@  class BBCooker:
                     #remove var from history
                     self.data.varhistory.del_var_history(var, conf_file, line)
 
-                total = ""
-                for c in contents:
-                    total += c
                 with open(conf_file, 'w') as f:
-                    f.write(total)
-                f.close()
+                    f.writelines(contents)
 
     def createConfigFile(self, name):
         path = os.getcwd()