Thread

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

    Marti Raudsepp <marti@juffo.org> — 2010-10-20T21:54:37Z

    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
    
  2. Re: psql: Don't close stdin, don't leak file descriptor with ON_ERROR_STOP

    Robert Haas <robertmhaas@gmail.com> — 2010-10-26T23:36:08Z

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