[PATCH] decodetree: Do not remove output_file from /dev

Richard Henderson posted 1 patch 11 months, 3 weeks ago
Failed in applying to current master (apply log)
scripts/decodetree.py | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[PATCH] decodetree: Do not remove output_file from /dev
Posted by Richard Henderson 11 months, 3 weeks ago
Nor report any PermissionError on remove.

Previously we were testing with "> /dev/null", but that's not easy
to do with meson test(), so we want to use '-o /dev/null' instead.
That works fine for all of the existing tests, where all errors are
diagnosed before opening the output file.  However, PMM's named field
patch set diagnoses cycle errors during output.  This is fair, but
we need to be more careful with the remove.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 scripts/decodetree.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index e4ef0a03cc..a9a0cd0fa3 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -71,7 +71,12 @@ def error_with_file(file, lineno, *args):
 
     if output_file and output_fd:
         output_fd.close()
-        os.remove(output_file)
+        # Do not try to remove e.g. -o /dev/null
+        if not output_file.startswith("/dev"):
+            try:
+                os.remove(output_file)
+            except PermissionError:
+                pass
     exit(0 if testforerror else 1)
 # end error_with_file
 
-- 
2.34.1
Re: [PATCH] decodetree: Do not remove output_file from /dev
Posted by Peter Maydell 11 months, 3 weeks ago
On Fri, 26 May 2023 at 18:40, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Nor report any PermissionError on remove.
>
> Previously we were testing with "> /dev/null", but that's not easy
> to do with meson test(), so we want to use '-o /dev/null' instead.
> That works fine for all of the existing tests, where all errors are
> diagnosed before opening the output file.  However, PMM's named field
> patch set diagnoses cycle errors during output.  This is fair, but
> we need to be more careful with the remove.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  scripts/decodetree.py | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/decodetree.py b/scripts/decodetree.py
> index e4ef0a03cc..a9a0cd0fa3 100644
> --- a/scripts/decodetree.py
> +++ b/scripts/decodetree.py
> @@ -71,7 +71,12 @@ def error_with_file(file, lineno, *args):
>
>      if output_file and output_fd:
>          output_fd.close()
> -        os.remove(output_file)
> +        # Do not try to remove e.g. -o /dev/null
> +        if not output_file.startswith("/dev"):
> +            try:
> +                os.remove(output_file)
> +            except PermissionError:
> +                pass

Maybe rather than hardcoding /dev, only try to delete the file
if it's a normal file, i.e.:
       if os.path.isfile(output_file):
           os.remove(output_file)

?

-- PMM
Re: [PATCH] decodetree: Do not remove output_file from /dev
Posted by Richard Henderson 11 months, 3 weeks ago
On 5/26/23 12:52, Peter Maydell wrote:
> On Fri, 26 May 2023 at 18:40, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Nor report any PermissionError on remove.
>>
>> Previously we were testing with "> /dev/null", but that's not easy
>> to do with meson test(), so we want to use '-o /dev/null' instead.
>> That works fine for all of the existing tests, where all errors are
>> diagnosed before opening the output file.  However, PMM's named field
>> patch set diagnoses cycle errors during output.  This is fair, but
>> we need to be more careful with the remove.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   scripts/decodetree.py | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/decodetree.py b/scripts/decodetree.py
>> index e4ef0a03cc..a9a0cd0fa3 100644
>> --- a/scripts/decodetree.py
>> +++ b/scripts/decodetree.py
>> @@ -71,7 +71,12 @@ def error_with_file(file, lineno, *args):
>>
>>       if output_file and output_fd:
>>           output_fd.close()
>> -        os.remove(output_file)
>> +        # Do not try to remove e.g. -o /dev/null
>> +        if not output_file.startswith("/dev"):
>> +            try:
>> +                os.remove(output_file)
>> +            except PermissionError:
>> +                pass
> 
> Maybe rather than hardcoding /dev, only try to delete the file
> if it's a normal file, i.e.:
>         if os.path.isfile(output_file):
>             os.remove(output_file)

Good idea.


r~