[OE-core] [PATCH 3/3] oeqa/utils/ftools: Checks before appending/reading files
Leonardo Sandoval
leonardo.sandoval.gonzalez at linux.intel.com
Mon Oct 19 14:21:18 UTC 2015
Hi Paul,
On 10/19/2015 08:41 AM, Paul Eggleton wrote:
> Hi Leo,
>
> On Monday 19 October 2015 05:24:44 leonardo.sandoval.gonzalez at linux.intel.com
> wrote:
>> From: Leonardo Sandoval <leonardo.sandoval.gonzalez at linux.intel.com>
>>
>> Before trying to append/read a file, check if file exists.
>>
>> Signed-off-by: Leonardo Sandoval
>> <leonardo.sandoval.gonzalez at linux.intel.com> ---
>> meta/lib/oeqa/utils/ftools.py | 24 ++++++++++++++----------
>> 1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/meta/lib/oeqa/utils/ftools.py b/meta/lib/oeqa/utils/ftools.py
>> index 64ebe3d..70a55b8 100644
>> --- a/meta/lib/oeqa/utils/ftools.py
>> +++ b/meta/lib/oeqa/utils/ftools.py
>> @@ -8,20 +8,24 @@ def write_file(path, data):
>>
>> def append_file(path, data):
>> wdata = data.rstrip() + "\n"
>> - with open(path, "a") as f:
>> + if os.path.isfile(path):
>> + with open(path, "a") as f:
>> f.write(wdata)
>
> Hang on - opening a nonexistent file with mode 'a' is perfectly fine, it'll just
> get created - why do we need this check?
You are right, there is no need.
>
>
>> def read_file(path):
>> data = None
>> - with open(path) as f:
>> - data = f.read()
>> + if os.path.isfile(path):
>> + with open(path) as f:
>> + data = f.read()
>> return data
>>
>> def remove_from_file(path, data):
>> - lines = read_file(path).splitlines()
>> - rmdata = data.strip().splitlines()
>> - for l in rmdata:
>> - for c in range(0, lines.count(l)):
>> - i = lines.index(l)
>> - del(lines[i])
>> - write_file(path, "\n".join(lines))
>> + rawdata = read_file(path)
>> + if rawdata:
>> + lines = rawdata.splitlines()
>> + rmdata = data.strip().splitlines()
>> + for l in rmdata:
>> + for c in range(0, lines.count(l)):
>> + i = lines.index(l)
>> + del(lines[i])
>> + write_file(path, "\n".join(lines))
>
> Checking a file exists before opening it isn't good practice. It's much better
> to try opening it and if the open fails with IOError of errno.ENOENT then
> ignore it (or rather, if e.errno != errno.ENOENT then re-raise the
> exception).
After applying patch into master
http://lists.openembedded.org/pipermail/openembedded-core/2015-October/111704.html
I saw this error:
Traceback (most recent call last):
File
"/srv/ab/yocto-autobuilder/yocto-worker/nightly-oe-selftest/build/meta/lib/oeqa/selftest/base.py",
line 110, in remove_config
ftools.remove_from_file(self.testinc_path, data)
File
"/srv/ab/yocto-autobuilder/yocto-worker/nightly-oe-selftest/build/meta/lib/oeqa/utils/ftools.py",
line 21, in remove_from_file
lines = read_file(path).splitlines()
File
"/srv/ab/yocto-autobuilder/yocto-worker/nightly-oe-selftest/build/meta/lib/oeqa/utils/ftools.py",
line 16, in read_file
with open(path) as f:
IOError: [Errno 2] No such file or directory:
'/srv/ab/yocto-autobuilder/yocto-worker/nightly-oe-selftest/build/build/conf/selftest.inc'
That is why I created the latter check. I will change it to use IOError,
safer and cleaner. Sending V2 today.
Thanks for your comments.
>
> Cheers,
> Paul
>
More information about the Openembedded-core
mailing list