Re: [bug-gv] gv-3.7.2: Possible pointless call to unlink ?

[Bernhard R. Link]

* David Binderman <address@hidden> [110611 20:30]:
> The source code is
> 
>     fclose(tmp_file);
>     unlink((char*) tmp_file);
> 
> Since the file is only read, I can't see any point in unlinking it.

The bug is much worse: the FILE pointer is given to unlink instead
of the name of the file to unlink. Additionally there is no check if
the fopen actually succeeded. Otherwise things simply get NULL pointers.

It makes sense to remove the file, as it was created earlier and
will not be deleted in this error case otherwise. (This code is
really hard to read, so hard it most likely loses more bugs than
it gains if it were to be rwritten).

I'd suggest something like (not tested):

From: Bernhard R. Link <address@hidden>
Date: Sun, 12 Jun 2011 12:03:10 +0200
Subject: [PATCH] handling dsc parse errors: fix FILE given to unlink, check for 
file open error

---
 gv/src/ps.c |   21 ++++++++++++---------
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/gv/src/ps.c b/gv/src/ps.c
index 9d2d7be..248081c 100644
--- a/gv/src/ps.c
+++ b/gv/src/ps.c
@@ -601,16 +601,19 @@ unc_ok:
        close(tmpfd);
 
         tmp_file = fopen( tmp_filename, "r" );
-       while ( fgets( password_line, 999, tmp_file) )
+        if (tmp_file)
        {
-          if (strstr(password_line,"This file requires a password for 
access."))
-             found = 1;
-          if (strstr(password_line,"Password did not work."))
-             found = 1;
-        }
-       fclose(tmp_file);       
-       unlink((char*) tmp_file);
-       
+         while ( fgets( password_line, 999, tmp_file) )
+         {
+            if (strstr(password_line,"This file requires a password for 
access."))
+               found = 1;
+            if (strstr(password_line,"Password did not work."))
+               found = 1;
+          }
+         fclose(tmp_file);
+         unlink(tmp_filename);
+       }
+
        if (found)
        {
           cb_askPassword((Widget)NULL, NULL, NULL);
-- 
1.7.2.5



        Bernhard R. Link