diff options
| author | Tom Lane | 2017-06-05 18:50:52 +0000 |
|---|---|---|
| committer | Tom Lane | 2017-06-05 18:50:59 +0000 |
| commit | 3e60c6f72328a9ad14d8a087411cd394752c5b23 (patch) | |
| tree | 5098f3109c3a1669278df7239cd2faebedaec84e /src/include | |
| parent | 614350a3ab73992f48c86e26552a2cbf030180e2 (diff) | |
Code review for shm_toc.h/.c.
Declare the toc_nentry field as uint32 not Size. Since shm_toc_lookup()
reads the field without any lock, it has to be atomically readable, and
we do not assume that for fields wider than 32 bits. Performance would
be impossibly bad for entry counts approaching 2^32 anyway, so there is
no need to try to preserve maximum width here.
This is probably an academic issue, because even if reading int64 isn't
atomic, the high order half would never change in practice. Still, it's
a coding rule violation, so let's fix it.
Adjust some other not-terribly-well-chosen data types too, and copy-edit
some comments. Make shm_toc_attach's Asserts consistent with
shm_toc_create's.
None of this looks to be a live bug, so no need for back-patch.
Discussion: https://postgr.es/m/16984.1496679541@sss.pgh.pa.us
Diffstat (limited to 'src/include')
| -rw-r--r-- | src/include/storage/shm_toc.h | 15 |
1 files changed, 8 insertions, 7 deletions
diff --git a/src/include/storage/shm_toc.h b/src/include/storage/shm_toc.h index 0548e309bd0..9175a472d88 100644 --- a/src/include/storage/shm_toc.h +++ b/src/include/storage/shm_toc.h @@ -22,9 +22,9 @@ #ifndef SHM_TOC_H #define SHM_TOC_H -#include "storage/shmem.h" +#include "storage/shmem.h" /* for add_size() */ -struct shm_toc; +/* shm_toc is an opaque type known only within shm_toc.c */ typedef struct shm_toc shm_toc; extern shm_toc *shm_toc_create(uint64 magic, void *address, Size nbytes); @@ -36,7 +36,9 @@ extern void *shm_toc_lookup(shm_toc *toc, uint64 key, bool noError); /* * Tools for estimating how large a chunk of shared memory will be needed - * to store a TOC and its dependent objects. + * to store a TOC and its dependent objects. Note: we don't really support + * large numbers of keys, but it's convenient to declare number_of_keys + * as a Size anyway. */ typedef struct { @@ -47,11 +49,10 @@ typedef struct #define shm_toc_initialize_estimator(e) \ ((e)->space_for_chunks = 0, (e)->number_of_keys = 0) #define shm_toc_estimate_chunk(e, sz) \ - ((e)->space_for_chunks = add_size((e)->space_for_chunks, \ - BUFFERALIGN((sz)))) + ((e)->space_for_chunks = add_size((e)->space_for_chunks, BUFFERALIGN(sz))) #define shm_toc_estimate_keys(e, cnt) \ - ((e)->number_of_keys = add_size((e)->number_of_keys, (cnt))) + ((e)->number_of_keys = add_size((e)->number_of_keys, cnt)) -extern Size shm_toc_estimate(shm_toc_estimator *); +extern Size shm_toc_estimate(shm_toc_estimator *e); #endif /* SHM_TOC_H */ |