On Sat, Jan 26, 2019 at 10:42 PM Jerry DeLisle <jvdeli...@charter.net>
wrote:

> Committed as simple and obvious. (With a ChangeLog Bobble fixed)
>
> Regression tested on x86_64.
>
> Committed r268301
>         M       libgfortran/ChangeLog
>         M       libgfortran/io/close.c
>
> Regards,
>
> Jerry
>
> 2019-01-26  Jerry DeLisle  <jvdeli...@gcc.gnu.org>
>
>         PR libfortran/89020
>         * io/close.c (st_close): Generate error if calls to 'remove'
>           return an error.
>
>
> diff --git a/libgfortran/io/close.c b/libgfortran/io/close.c
> index cbcbf4e71a1..2b35e49c9cc 100644
> --- a/libgfortran/io/close.c
> +++ b/libgfortran/io/close.c
> @@ -99,7 +99,11 @@ st_close (st_parameter_close *clp)
>               else
>                 {
>   #if HAVE_UNLINK_OPEN_FILE
> -                 remove (u->filename);
> +
> +                 if (remove (u->filename))
> +                   generate_error (&clp->common, LIBERROR_OS,
> +                           "File cannot be deleted, possibly in use by"
> +                           " another process");
>   #else
>                   path = strdup (u->filename);
>   #endif
> @@ -112,7 +116,10 @@ st_close (st_parameter_close *clp)
>   #if !HAVE_UNLINK_OPEN_FILE
>         if (path != NULL)
>         {
> -         remove (path);
> +         if (remove (u->filename))
> +           generate_error (&clp->common, LIBERROR_OS,
> +                   "File cannot be deleted, possibly in use by"
> +                   " another process");
>           free (path);
>         }
>   #endif
>


Checking remove() for an error is a good idea, although speculating why
that happened might be confusing if the error happens for some other
reason? Particularly so on POSIX systems, where deleting open files is
explicitly part of the design.  In generate_error(), we already call
gf_strerror() to translate errno to a string, which should give a more
accurate description of what went wrong? So maybe just say "File cannot be
deleted"?

And yes, as mentioned in the PR, the second remove should "remove (path)"
and not "u->filename".

-- 
Janne Blomqvist

Reply via email to