Re: [HACKERS] psql -f doesn't complain about directories
Am Donnerstag, 15. November 2007 schrieb Tom Lane:
Peter Eisentraut [EMAIL PROTECTED] writes:
Am Donnerstag, 15. November 2007 schrieb Tom Lane:
This seems too far removed from the scene of the crime
Yeah, my zeroth attempt was to place this in gets_fromFile(), but there
you don't have any opportunity to report failure to the main loop. We'd
need to change the function signature to be able to pass that around.
Maybe that's better overall.
Well, you could still handle that the same as in your patch: on NULL
return, check ferror. It's just that I don't trust errno to stay
unchanged for very long.
This should do better:
diff -ur ../cvs-pgsql/src/bin/psql/input.c ./src/bin/psql/input.c
--- ../cvs-pgsql/src/bin/psql/input.c 2007-01-12 10:22:42.0 +0100
+++ ./src/bin/psql/input.c 2007-11-27 18:46:34.0 +0100
@@ -179,9 +179,16 @@
/* Disable SIGINT again */
sigint_interrupt_enabled = false;
- /* EOF? */
+ /* EOF or error? */
if (result == NULL)
+ {
+ if (ferror(source))
+ {
+ psql_error(could not read from input file:
%s\n, strerror(errno));
+ return NULL;
+ }
break;
+ }
appendPQExpBufferStr(buffer, line);
diff -ur ../cvs-pgsql/src/bin/psql/mainloop.c ./src/bin/psql/mainloop.c
--- ../cvs-pgsql/src/bin/psql/mainloop.c2007-01-12 10:22:42.0
+0100
+++ ./src/bin/psql/mainloop.c 2007-11-27 18:30:13.0 +0100
@@ -129,7 +129,11 @@
line = gets_interactive(get_prompt(prompt_status));
}
else
+ {
line = gets_fromFile(source);
+ if (!line ferror(source))
+ successResult = EXIT_FAILURE;
+ }
/*
* query_buf holds query already accumulated. line is the
malloc'd
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [HACKERS] psql -f doesn't complain about directories
Peter Eisentraut [EMAIL PROTECTED] writes: This should do better: Looks good to me, though I'd suggest updating gets_fromFile's header comment: - * The result is a malloc'd string. + * The result is a malloc'd string, or NULL on EOF or input error. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] psql -f doesn't complain about directories
Martijn van Oosterhout napsal(a): On Wed, Nov 14, 2007 at 09:33:17PM +0100, Zdenek Kotala wrote: Sure, why not. To be honest I think that psql shouldn't be ignoring the EISDIR error the kernel is returning. But it works when you open directory in read-only mode. See posix definition: [EISDIR] The named file is a directory and oflag includes O_WRONLY or O_RDWR. $ strace psql -f /tmp snip open(/tmp, O_RDONLY|O_LARGEFILE) = 4 snip read(4, 0xb7f1b000, 4096) = -1 EISDIR (Is a directory) Which is subsequently ignored. I'm hoping it doesn't ignore other errors, like EIO or EPIPE, Yes, you have right I checked only open command which works fine, but read fails. Zdenek ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] psql -f doesn't complain about directories
Am Mittwoch, 14. November 2007 schrieb Martijn van Oosterhout:
It's not the fopen that fails, it's the fgets that returns NULL. We
don't subsequently check if that's due to an I/O error or EISDIR or if
it's an end-of-file.
Here is a patch for this.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
diff -ru ../../../../cvs-pgsql/src/bin/psql/input.c ./input.c
--- ../../../../cvs-pgsql/src/bin/psql/input.c 2007-01-05 23:19:49.0 +0100
+++ ./input.c 2007-11-15 13:59:22.0 +0100
@@ -179,7 +179,7 @@
/* Disable SIGINT again */
sigint_interrupt_enabled = false;
- /* EOF? */
+ /* EOF or error? */
if (result == NULL)
break;
diff -ru ../../../../cvs-pgsql/src/bin/psql/mainloop.c ./mainloop.c
--- ../../../../cvs-pgsql/src/bin/psql/mainloop.c 2007-01-05 23:19:49.0 +0100
+++ ./mainloop.c 2007-11-15 13:57:36.0 +0100
@@ -129,7 +129,14 @@
line = gets_interactive(get_prompt(prompt_status));
}
else
+ {
line = gets_fromFile(source);
+ if (!line ferror(source))
+ {
+psql_error(could not read from input file: %s\n, strerror(errno));
+successResult = EXIT_FAILURE;
+ }
+ }
/*
* query_buf holds query already accumulated. line is the malloc'd
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [HACKERS] psql -f doesn't complain about directories
Peter Eisentraut [EMAIL PROTECTED] writes: Am Mittwoch, 14. November 2007 schrieb Martijn van Oosterhout: It's not the fopen that fails, it's the fgets that returns NULL. We don't subsequently check if that's due to an I/O error or EISDIR or if it's an end-of-file. Here is a patch for this. This seems too far removed from the scene of the crime --- I don't have a lot of confidence that errno will still be unchanged back in the main loop. I'd rather see the psql_error printout occur immediately after the failed fgets call. Either that or you need to be a bit more proactive about ensuring errno is returned undamaged. Also, I think you overlooked the case where we get a read error after having already loaded some data into gets_fromFile's result buffer. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] psql -f doesn't complain about directories
Am Donnerstag, 15. November 2007 schrieb Tom Lane: This seems too far removed from the scene of the crime Yeah, my zeroth attempt was to place this in gets_fromFile(), but there you don't have any opportunity to report failure to the main loop. We'd need to change the function signature to be able to pass that around. Maybe that's better overall. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] psql -f doesn't complain about directories
Peter Eisentraut [EMAIL PROTECTED] writes: Am Donnerstag, 15. November 2007 schrieb Tom Lane: This seems too far removed from the scene of the crime Yeah, my zeroth attempt was to place this in gets_fromFile(), but there you don't have any opportunity to report failure to the main loop. We'd need to change the function signature to be able to pass that around. Maybe that's better overall. Well, you could still handle that the same as in your patch: on NULL return, check ferror. It's just that I don't trust errno to stay unchanged for very long. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] psql -f doesn't complain about directories
On Wed, Nov 14, 2007 at 05:15:20PM -0300, Alvaro Herrera wrote: Should we do some kind of stat() before opening the file and abort if it's a directory? Actually anything other than a plain file, right? (Do we really want to be able to psql -f a_pipe?) Sure, why not. To be honest I think that psql shouldn't be ignoring the EISDIR error the kernel is returning. Have a nice day, -- Martijn van Oosterhout [EMAIL PROTECTED] http://svana.org/kleptog/ Those who make peaceful revolution impossible will make violent revolution inevitable. -- John F Kennedy signature.asc Description: Digital signature
Re: [HACKERS] psql -f doesn't complain about directories
Martijn van Oosterhout wrote: On Wed, Nov 14, 2007 at 05:15:20PM -0300, Alvaro Herrera wrote: Should we do some kind of stat() before opening the file and abort if it's a directory? Actually anything other than a plain file, right? (Do we really want to be able to psql -f a_pipe?) Sure, why not. To be honest I think that psql shouldn't be ignoring the EISDIR error the kernel is returning. But it works when you open directory in read-only mode. See posix definition: [EISDIR] The named file is a directory and oflag includes O_WRONLY or O_RDWR. Zdenek ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] psql -f doesn't complain about directories
Alvaro Herrera wrote: Peter Eisentraut wrote: Letting psql execute a script file that is really a directory doesn't complain at all: $ psql -f /tmp Should we do some kind of stat() before opening the file and abort if it's a directory? Actually anything other than a plain file, right? (Do we really want to be able to psql -f a_pipe?) What's about symlink to regular file/pipe? Zdenek ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] psql -f doesn't complain about directories
Peter Eisentraut wrote: Letting psql execute a script file that is really a directory doesn't complain at all: $ psql -f /tmp Should we do some kind of stat() before opening the file and abort if it's a directory? Actually anything other than a plain file, right? (Do we really want to be able to psql -f a_pipe?) -- Alvaro Herrera Developer, http://www.PostgreSQL.org/ Essentially, you're proposing Kevlar shoes as a solution for the problem that you want to walk around carrying a loaded gun aimed at your foot. (Tom Lane) ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] psql -f doesn't complain about directories
On Wed, Nov 14, 2007 at 09:33:17PM +0100, Zdenek Kotala wrote: Sure, why not. To be honest I think that psql shouldn't be ignoring the EISDIR error the kernel is returning. But it works when you open directory in read-only mode. See posix definition: [EISDIR] The named file is a directory and oflag includes O_WRONLY or O_RDWR. $ strace psql -f /tmp snip open(/tmp, O_RDONLY|O_LARGEFILE) = 4 snip read(4, 0xb7f1b000, 4096) = -1 EISDIR (Is a directory) Which is subsequently ignored. I'm hoping it doesn't ignore other errors, like EIO or EPIPE, Have a nice day, -- Martijn van Oosterhout [EMAIL PROTECTED] http://svana.org/kleptog/ Those who make peaceful revolution impossible will make violent revolution inevitable. -- John F Kennedy signature.asc Description: Digital signature
Re: [HACKERS] psql -f doesn't complain about directories
Alvaro Herrera wrote: Peter Eisentraut wrote: Letting psql execute a script file that is really a directory doesn't complain at all: $ psql -f /tmp Should we do some kind of stat() before opening the file and abort if it's a directory? Actually anything other than a plain file, right? (Do we really want to be able to psql -f a_pipe?) I don't see why not. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] psql -f doesn't complain about directories
Martijn van Oosterhout wrote: To be honest I think that psql shouldn't be ignoring the EISDIR error the kernel is returning. We use fopen(), which doesn't appear to pass that on. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] psql -f doesn't complain about directories
On Wed, Nov 14, 2007 at 10:25:23PM +0100, Peter Eisentraut wrote: Martijn van Oosterhout wrote: To be honest I think that psql shouldn't be ignoring the EISDIR error the kernel is returning. We use fopen(), which doesn't appear to pass that on. It's not the fopen that fails, it's the fgets that returns NULL. We don't subsequently check if that's due to an I/O error or EISDIR or if it's an end-of-file. Have a nice day, -- Martijn van Oosterhout [EMAIL PROTECTED] http://svana.org/kleptog/ Those who make peaceful revolution impossible will make violent revolution inevitable. -- John F Kennedy signature.asc Description: Digital signature
Re: [HACKERS] psql -f doesn't complain about directories
On Wed, Nov 14, 2007 at 05:15:20PM -0300, Alvaro Herrera wrote: Peter Eisentraut wrote: Letting psql execute a script file that is really a directory doesn't complain at all: $ psql -f /tmp Should we do some kind of stat() before opening the file and abort if it's a directory? Actually anything other than a plain file, right? (Do we really want to be able to psql -f a_pipe?) Yes, I have seen people use just this technique. Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] psql -f doesn't complain about directories
David Fetter wrote: On Wed, Nov 14, 2007 at 05:15:20PM -0300, Alvaro Herrera wrote: Peter Eisentraut wrote: Letting psql execute a script file that is really a directory doesn't complain at all: $ psql -f /tmp Should we do some kind of stat() before opening the file and abort if it's a directory? Actually anything other than a plain file, right? (Do we really want to be able to psql -f a_pipe?) Yes, I have seen people use just this technique. Interesting. Why not just use a standard shell pipe from the command writing into the named pipe, instead of piping through it? -- Alvaro Herrera http://www.flickr.com/photos/alvherre/ Having your biases confirmed independently is how scientific progress is made, and hence made our great society what it is today (Mary Gardiner) ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
