Korry spotted two violations of the rule about accessing the first varlena
field directly which I had missed. In both cases I believe the accesses are
actually safe because they follow an int4 column and the data are either
explicitly detoasted or passed to textout which can handle 1-byte headers

One instance is in flatfiles.c when it accesses the password field which is a
text field. Here I don't see any reasonable way to avoid it and think we
should just document that we're bending the rules.

The other instance is in inv_api.c where it would be quite possible to use
fastgetattr() instead. But the column is always at the same fixed offset and
again it follows an int4 so it'll always be 4-byte aligned and work fine. So
for performance reasons perhaps we should keep this as well?

I've attached three patches. A patch which adds a comment for flatfiles.c. And
two patches for inv_api.c one which just adds a comment and one which converts
it to use fastgetattr().

Again, credit to Korry for spotting these. To do so he commented out all the
varlena fields (excluding int2vector and oidvector) from the struct
definitions and saw what broke. 

Why do we even have those fields in the structs if they're unsafe to use?
Perhaps we should #if 0 out all the unsafe fields from all the struct
definitions? That would avoid one of the most common new programmer bugs and
also let us document the few exceptions and why they're allowed.

Attachment: inv_api.c-minimal.gz
Description: Binary data

Attachment: flatfiles.c-minimal.gz
Description: Binary data

Attachment: inv_api.c-fastgetattr.gz
Description: Binary data

  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com
---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Reply via email to