Re: [HACKERS] Error handling for ShmemInitStruct and ShmemInitHash
Tom Lane wrote: The functions ShmemInitStruct and ShmemInitHash will return NULL on certain failure conditions, apparently on the grounds that their caller can print a more useful error message than they can. A quick survey shows that about half the callers aren't remembering to check for NULL, and none of the other half are printing messages that are more useful than out of shared memory (which isn't even necessarily correct). I think that this is pretty error-prone, and that considering that PG hackers are accustomed to not checking palloc() results, it's inevitable that we'll make the same mistake in future if we leave this API as it is. I suggest making these functions throw their own errors rather than returning NULL on failure, and removing the redundant error reports from the callers that have 'em. +1. I was just annoyed by this when working on the known-assigned-xids hash - sorted-array patch. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Error handling for ShmemInitStruct and ShmemInitHash
Tom Lane t...@sss.pgh.pa.us wrote: none of the other half are printing messages that are more useful than out of shared memory (which isn't even necessarily correct). I think the messages in the locking area are a bit more useful than out of shared memory, but it would be trivial to build the equivalent message in the ShmemInitHash function, based on the first parameter. LockMethodProcLockHash = ShmemInitHash(PROCLOCK hash, init_table_size, max_table_size, info, hash_flags); if (!LockMethodProcLockHash) elog(FATAL, could not initialize proclock hash table); Presumably the ShmemInitHash function could add other information which would make the message *more* useful. (Perhaps other parameter information or maybe even the actual *cause* of the failure.) I suggest making these functions throw their own errors rather than returning NULL on failure, and removing the redundant error reports from the callers that have 'em. +1 It would be low priority if the return value was currently being consistently checked for NULL; but since that's not the case we have to do something, and what you suggest sounds best, long term. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Error handling for ShmemInitStruct and ShmemInitHash
Kevin Grittner kevin.gritt...@wicourts.gov writes: Tom Lane t...@sss.pgh.pa.us wrote: none of the other half are printing messages that are more useful than out of shared memory (which isn't even necessarily correct). I think the messages in the locking area are a bit more useful than out of shared memory, but it would be trivial to build the equivalent message in the ShmemInitHash function, based on the first parameter. Right, I was intending to include the name parameter in the messages. This would actually represent an improvement in message quality in a lot of the cases. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Error handling for ShmemInitStruct and ShmemInitHash
The functions ShmemInitStruct and ShmemInitHash will return NULL on certain failure conditions, apparently on the grounds that their caller can print a more useful error message than they can. A quick survey shows that about half the callers aren't remembering to check for NULL, and none of the other half are printing messages that are more useful than out of shared memory (which isn't even necessarily correct). I think that this is pretty error-prone, and that considering that PG hackers are accustomed to not checking palloc() results, it's inevitable that we'll make the same mistake in future if we leave this API as it is. I suggest making these functions throw their own errors rather than returning NULL on failure, and removing the redundant error reports from the callers that have 'em. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
