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

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Marti Raudsepp <marti(at)juffo(dot)org>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: psql: Don't close stdin, don't leak file descriptor with ON_ERROR_STOP
Date: 2010-10-26 23:36:08
Message-ID: AANLkTi=uPxQVP9ipc6Aj9GKg2=ZwDtD1mb22iRZsr6Nn@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Oct 20, 2010 at 5:54 PM, Marti Raudsepp <marti(at)juffo(dot)org> wrote:
> 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.

Thanks, committed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2010-10-27 00:45:25 Re: add label to enum syntax
Previous Message Merlin Moncure 2010-10-26 23:16:53 Re: Postgres insert performance and storage requirement compared to Oracle