Re: make 'PrivateRefCount' 32 bits wide

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