John Naylor <john.naylor@enterprisedb.com> writes:
> It seems that we should have "Assert(word != 0);" at the top, which matches
> the other platforms anyway, so I'll add that.
That's basically equivalent to the existing Assert(non_zero).
I think it'd be okay to drop that one and instead have
the same Assert condition as other platforms, but having both
would be redundant.
I agree that adding the non-Assert test that Ranier wants
is entirely pointless. If a caller did manage to violate
the asserted-for condition (and we don't have asserts on),
returning zero is not better than returning an unspecified
value. If anything it might be worse, since it might not
lead to obvious misbehavior.
On the whole, there's not anything wrong with the code as-is.
A case could be made for making the MSVC asserts more like
other platforms, but it's quite cosmetic.
regards, tom lane