On Thu, May 8, 2014 at 10:28 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> Well, we have to live with it for now :)
I just had a look at the first patch and got some comments:
1) Instead of using an assertion here, wouldn't it be better to error
out if name is NULL, and truncate the name if it is longer than
SHMEM_INDEX_KEYSIZE - 1 (including '\0')?
scanstr in scansup.c? Assert(IsUnderPostmaster);
+ Assert(name != NULL && strlen(name) > 0 &&
+ strlen(name) < SHMEM_INDEX_KEYSIZE - 1);
2) The addition of a field to track the size of a dsm should be
explicitly mentioned, this is useful for the 2nd patch.
3) The refactoring done in dsm_create to find an unused slot should be
done as a separate patch for clarity.
4) Using '\0' here would be more adapted:
+ item->name[SHMEM_INDEX_KEYSIZE - 1] = 0;
Regards,
--
Michael