Re: Is this non-volatile pointer access OK?

On 3 September 2012 08:10, Daniel Farina <daniel(at)heroku(dot)com> wrote:
> http://doxygen.postgresql.org/xlog_8c_source.html#l08197
>
> On line 8197 of xlog.c:
>
> 08194 /* Get a local copy of the last safe checkpoint record. */
> 08195 SpinLockAcquire(&xlogctl->info_lck);
> 08196 lastCheckPointRecPtr = xlogctl->lastCheckPointRecPtr;
> 08197 memcpy(&lastCheckPoint, &XLogCtl->lastCheckPoint, sizeof(CheckPoint));
> 08198 SpinLockRelease(&xlogctl->info_lck);
>
> Note the use of capital XLogCtl->lastCheckPoint, which is not the
> volatile pointer.

That looks like a bug to me.

Come to think of it, the whole convention of using a lower-case
variant of the original pointer variable name seems like a foot-gun,
given the harmful and indeed very subtle consequences of making this
error.

I count 98 SpinLockAcquire() call sites (of which only a minority use
this convention, which is mostly within xlog.c, I think). Is it worth
instituting an alternative convention to make this kind of misuse more
obvious? This went unnoticed since February 2009.

--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services