Re: MemSetLoop ignoring the 'val' parameter

Hi,

#define MemSetLoop(start, val, len) \
do \
{ \
long * _start = (long *) (start); \
long * _stop = (long *) ((char *) _start + (Size) (len)); \
\
while (_start < _stop) \
*_start++ = 0; \
} while (0)

The 'val' parameter is ignored.

Currently it doesn't matter because MemSetLoop is only used with a 0 parameter
and only so in mcxt.c but it looks like it should be fixed anyway.

Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Monday, October 08, 2012 10:39:27 PM Andres Freund wrote:
> The 'val' parameter is ignored.
Trivial patch attached.
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> The 'val' parameter is ignored.

This is not broken. Read the comments for MemSetTest.

regards, tom lane

On Tuesday, October 09, 2012 12:56:16 AM Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > The 'val' parameter is ignored.
>
> This is not broken. Read the comments for MemSetTest.
Ah. I was surprised about that already. The comment says that val has to be
constant though, not that it has to be zero. In my understanding 1 is constant
as well. Also, why do we even pass in a 'val' parameter in that case?

Feeling a little slow here...

Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On Tuesday, October 09, 2012 12:56:16 AM Tom Lane wrote:
>> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>>> The 'val' parameter is ignored.

>> This is not broken. Read the comments for MemSetTest.

> Ah. I was surprised about that already. The comment says that val has to be
> constant though, not that it has to be zero. In my understanding 1 is constant
> as well. Also, why do we even pass in a 'val' parameter in that case?

Well, first off, the callers should not be aware of the detail that
MemSetTest insists on a val of zero, so they have to pass val even
though it's unused by the current implementation of MemSetLoop.

The callers are responsible for not passing a volatile value there, but
it's hard to dodge that problem given that we're dealing with macros;
if the value changes on repeat evaluation we're screwed anyway.

However, "nonvolatile" is not "constant". For instance, it's perfectly
fine to pass MemSetTest/Loop a variable for the "val" that is sometimes
zero and sometimes not. If we changed the coding as you suggest, the
compiler would probably generate less efficient code since it wouldn't
realize (unless it was quite smart) that MemSetLoop is always filling
with zeroes.

regards, tom lane