* David Binderman <[email protected]> [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 <[email protected]> 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
