On Fri, Jan  9, 2015 at 11:34:24AM -0500, Tom Lane wrote:
> Tatsuo Ishii <is...@postgresql.org> writes:
> > According to Coverity, there's a memory leak bug in transfer_all_new_dbs().
> 
> It's pretty difficult to get excited about that; how many table-free
> databases is pg_upgrade likely to see in one run?  But surely we could
> just move the pg_free call to after the if-block.

I have fixed this with the attached, applied patch.  I thought malloc(0)
would return null, but our src/common pg_malloc() has:

    /* Avoid unportable behavior of malloc(0) */
    if (size == 0)
        size = 1;

so some memory is allocated, and has to be freed.  I looked at avoiding
the call to gen_db_file_maps() for old_db->rel_arr.nrels == 0, but there
are checks in there comparing the old/new relation counts, so it can't
be skipped.  I also removed the unnecessary memory initialization.

-- 
  Bruce Momjian  <br...@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +
commit ac7009abd228362042edd10e6b12556ddef35171
Author: Bruce Momjian <br...@momjian.us>
Date:   Fri Jan 9 12:12:30 2015 -0500

    pg_upgrade:  fix one-byte per empty db memory leak
    
    Report by Tatsuo Ishii, Coverity

diff --git a/contrib/pg_upgrade/relfilenode.c b/contrib/pg_upgrade/relfilenode.c
new file mode 100644
index 70753f2..423802b
*** a/contrib/pg_upgrade/relfilenode.c
--- b/contrib/pg_upgrade/relfilenode.c
*************** transfer_all_new_dbs(DbInfoArr *old_db_a
*** 110,119 ****
                        pg_fatal("old database \"%s\" not found in the new 
cluster\n",
                                         old_db->db_name);
  
-               n_maps = 0;
                mappings = gen_db_file_maps(old_db, new_db, &n_maps, old_pgdata,
                                                                        
new_pgdata);
- 
                if (n_maps)
                {
                        print_maps(mappings, n_maps, new_db->db_name);
--- 110,117 ----
*************** transfer_all_new_dbs(DbInfoArr *old_db_a
*** 123,131 ****
  #endif
                        transfer_single_new_db(pageConverter, mappings, n_maps,
                                                                   
old_tablespace);
- 
-                       pg_free(mappings);
                }
        }
  
        return;
--- 121,129 ----
  #endif
                        transfer_single_new_db(pageConverter, mappings, n_maps,
                                                                   
old_tablespace);
                }
+               /* We allocate something even for n_maps == 0 */
+               pg_free(mappings);
        }
  
        return;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to