Thread: 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.