diff mbox series

decodetree: Do not remove output_file from /dev

Message ID 20230526174005.1801043-1-richard.henderson@linaro.org
State Superseded
Headers show
Series decodetree: Do not remove output_file from /dev | expand

Commit Message

Richard Henderson May 26, 2023, 5:40 p.m. UTC
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(-)

Comments

Peter Maydell May 26, 2023, 7:52 p.m. UTC | #1
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
Richard Henderson May 26, 2023, 9:49 p.m. UTC | #2
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~
diff mbox series

Patch

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