psql: Don't close stdin, don't leak file descriptor with ON_ERROR_STOP

From: Marti Raudsepp <marti(at)juffo(dot)org>
To: pgsql-hackers(at)postgresql(dot)org
Subject: psql: Don't close stdin, don't leak file descriptor with ON_ERROR_STOP
Date: 2010-10-20 21:54:37
Message-ID: AANLkTi=033=6jo=vCLCfhkXVWVn8H6y4Oq1yNpAVOeex@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi list,

Here's the second patch from my coccicheck run. Originally it flagged
the fact that the opened file in psql's process_file() wasn't being
closed in the ON_ERROR_STOP path, but there seem to be two more
unintended behaviors here.

(1) In the error path, the value of pset.inputfile wasn't being
properly restored. The caller does free(fname) on line 786, so
psql.inputfile would point to unallocated memory.

(2) The more significant issue is that stdin *was closed in the
success return path. So when you run a script with two "\i -" lines,
the first "\q" would close stdin and the next one would fail with:
psql:-:0: could not read from input file: Bad file descriptor

In fact, this means that stdin was being accessed after being
fclose()d, which is undefined behavior per ANSI C, though it seems to
work just fine on Linux.

The new behavior requires the same amount of "\q"s as the number of
executions of '-' because stdin is never closed.

Regards,
Marti

Attachment Content-Type Size
0001-psql-Don-t-close-stdin-don-t-leak-file-descriptor-wi.patch text/x-patch 1.7 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dimitri Fontaine 2010-10-20 21:56:44 Re: Extensions, this time with a patch
Previous Message Stephen R. van den Berg 2010-10-20 21:30:50 Re: pg_rawdump