Обсуждение: question on bits32 wraparound check

So there's a minor issue on Takahiro-san fillfactor-on-toast patch,
which is that it does not hand out the last possible "kind" value.  This
is a bits32 field, so at least theoretically on some platforms it will
be wider than 32 while on others it will be exactly 32.  I'm wondering
if this is the correct way to check for wraparound:

relopt_kind
add_reloption_kind(void)
{relopt_kind        kind;
/* wraparound check */if (last_assigned_kind > RELOPT_KIND_MAX ||    last_assigned_kind == 0)    ereport(ERROR,
  (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),             errmsg("user-defined relation parameter types limit
exceeded")));
kind = (relopt_kind) last_assigned_kind;last_assigned_kind <<= 1;return kind;
}


Fixing this I'm ready to commit this patch.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre@commandprompt.com> writes:
> So there's a minor issue on Takahiro-san fillfactor-on-toast patch,
> which is that it does not hand out the last possible "kind" value.  This
> is a bits32 field, so at least theoretically on some platforms it will
> be wider than 32 while on others it will be exactly 32.

So just disallow kind > 31 (hard wired, with a note cross-referencing
the datatype definition).  Even if that leaves some unused bits on some
particular platform, it's correct and ensures portable behavior.
        regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > So there's a minor issue on Takahiro-san fillfactor-on-toast patch,
> > which is that it does not hand out the last possible "kind" value.  This
> > is a bits32 field, so at least theoretically on some platforms it will
> > be wider than 32 while on others it will be exactly 32.
> 
> So just disallow kind > 31 (hard wired, with a note cross-referencing
> the datatype definition).  Even if that leaves some unused bits on some
> particular platform, it's correct and ensures portable behavior.

So Takahiro-san got it right the first time.  Thanks, committed that
way.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.