Re: make 'PrivateRefCount' 32 bits wide

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: make 'PrivateRefCount' 32 bits wide
Date: 2004-04-22 03:20:56
Message-ID: 18715.1082604056@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Neil Conway <neilc(at)samurai(dot)com> writes:
> This patch changes PrivateRefCount and LocalRefCount in the bufmgr from
> being arrays of long to arrays of int32, per earlier discussion.

I just committed a bunch of changes in bufmgr --- hope I didn't tramp on
your toes too much.

> *** 176,184 ****
> /*
> * Allocate and zero local arrays of per-buffer info.
> */
> ! BufferBlockPointers = (Block *) calloc(NBuffers, sizeof(Block));
> ! PrivateRefCount = (long *) calloc(NBuffers, sizeof(long));
> ! BufferLocks = (bits8 *) calloc(NBuffers, sizeof(bits8));

> /*
> * Convert shmem offsets into addresses as seen by this process. This
> --- 176,184 ----
> /*
> * Allocate and zero local arrays of per-buffer info.
> */
> ! BufferBlockPointers = calloc(NBuffers, sizeof(*BufferBlockPointers));
> ! PrivateRefCount = calloc(NBuffers, sizeof(*PrivateRefCount));
> ! BufferLocks = calloc(NBuffers, sizeof(*BufferLocks));

This I think is a bad change. The coding style
ptr_var = (foo *) alloc(n * sizeof(foo));
is widely used in the backend, and I consider it good style because you
will get a warning if ptr_var is a pointer to something other than foo.
Removing the cast eliminates a significant protection against the risk
of using the wrong datatype in the sizeof calculation. I realize that
if you mistakenly write
ptr_var = (foo *) alloc(n * sizeof(bar));
then you're screwed anyway --- but this is a localized mistake. Since
the declaration of ptr_var might be quite far from the allocation
statement, it's not hard to mess up ptr_var versus foo, whereas two type
references in the same statement are more likely to track each other.

As an example, your change to make PrivateRefCount be int32 * instead of
long * would draw no warning at all if the allocation command failed to
be changed to match, if the command were written in the cast-less way.

regards, tom lane

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2004-04-22 03:49:11 Re: contrib/dbmirror
Previous Message Bruce Momjian 2004-04-22 02:53:58 Re: [JDBC] EXECUTE command tag returns actual command