Re: [pgsql-patches] Patch to avoid gprofprofilingoverwrites

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Nikhil S <nikhil(dot)sontakke(at)enterprisedb(dot)com>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Korry Douglas <korryd(at)enterprisedb(dot)com>
Subject: Re: [pgsql-patches] Patch to avoid gprofprofilingoverwrites
Date: 2007-02-21 01:45:15
Message-ID: 200702210145.l1L1jFX20751@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>
> > + CFLAGS="$CFLAGS -DPROFILE_PID_DIR -pg ${PROFILE_CFLAGS}"
>
> Kindly use AC_DEFINE instead of random -D in CFLAGS (which is the wrong
> place for -D anyway). Also, what exactly is the point here of
> PROFILE_CFLAGS? I thought it was supposed to allow substituting
> something else for -pg, but you've managed to defeat that.

I can't see the value in having a profile flag that just adds an
environment variable. I am hoping other compilers will supply the flags
they need and we can expand this.

> > + snprintf(gprofDirName, MAXPGPATH, "./gprof/%d", getpid());
>
> getpid is not int everywhere; use a cast. Also, the "./" bits are
> silly, and if you ask me so is the MAXPGPATH-sized buffer for a string
> that can't exceed 20 or so bytes.

Patch updated and attached.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Attachment Content-Type Size
/pgpatches/profile text/x-diff 10.2 KB

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Joshua D. Drake 2007-02-21 03:24:52 Re: tsearch in core patch, for inclusion
Previous Message Tom Lane 2007-02-21 01:08:18 Re: WIP patch - INSERT-able log statements