Re: Win32: Minimising desktop heap usage

From: Dave Page <dpage(at)postgresql(dot)org>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: Win32: Minimising desktop heap usage
Date: 2007-10-23 15:47:33
Message-ID: 471E1795.2030207@postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Tom Lane wrote:
> Dave Page <dpage(at)postgresql(dot)org> writes:
>> [ get rid of wsprintf in favor of snprintf ]
>
> +1 for not depending on nonstandard subroutines without need.
> But please note the standard locution is snprintf(buf, sizeof(buf), ...
> Not sizeof() - 1.

Noted.

>> ! char *tmppath=0;
>
> Please use NULL not 0 ... actually, since the variable is
> unconditionally assigned to just below, I'd say this should just be
> "char *tmppath;". I don't like useless initializations: if the compiler
> is not smart enough to discard them then they waste cycles, and they
> also increase the risk of bugs-of-omission in future changes. If
> someone were to change things around so that tmppath didn't get assigned
> to in one code path, then the compiler would complain about use of an
> uninitialized variable --- *unless* you've written a useless
> initialization such as the above, in which case the mistake might pass
> unnoticed for awhile. So don't initialize a local variable unless
> you're giving it an actually meaningful value.

The downside is that we see a useless warning that, if sufficiently
multiplied, might make it hard to see something we are interested in.

In this case, not initialising it will also create the first 'accepted'
warning in VC++ in our code :-(. All the others come from Kerberos.

Modified in the attached patch however.

>> ! /*
>> ! * Note: We use getenv here because the more modern SHGetSpecialFolderPath()
>> ! * will force us to link with shell32.lib which eats valuable desktop heap.
>> ! */
>> ! tmppath = getenv("APPDATA");
>
> Hmm, is there any functional downside to this? I suppose
> SHGetSpecialFolderPath does some things that getenv does not...

A good percentage of the special folder paths you might query with
SHGetSpecialFolderPath() are not actually in the environment. APPDATA
certainly is on XP, 2K3 and Vista though, and I've found MS KB articles
referring to it being on 2K as well.

Regards, Dave.

Attachment Content-Type Size
desktop_heap2.patch text/x-diff 3.1 KB

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2007-10-23 15:54:16 Re: Win32: Minimising desktop heap usage
Previous Message Tom Lane 2007-10-23 15:30:03 Re: Win32: Minimising desktop heap usage