Обсуждение: Questionable codes
I have found a few questionable codings. I'm not sure if it really hurts anything. Suggestions are welcome. 1) in storage/lmgr/lock.c: LockShmemSize() size += MAXALIGN(maxBackends * sizeof(PROC)); /* each MyProc */ size += MAXALIGN(maxBackends * sizeof(LOCKMETHODCTL)); /* each shouldn't be: size += maxBackends * MAXALIGN(sizeof(PROC)); /* each MyProc */ size += maxBackends * MAXALIGN(sizeof(LOCKMETHODCTL)); /* each 2) in utils/hash/dynahash.c:hash_search(): Assert(saveState.currElem && !(saveState.currElem = 0)); Does anybody know what it is for? -- Tatsuo Ishii
> I have found a few questionable codings. I'm not sure if it really > hurts anything. Suggestions are welcome. > > 1) in storage/lmgr/lock.c: LockShmemSize() > > size += MAXALIGN(maxBackends * sizeof(PROC)); /* each MyProc */ > size += MAXALIGN(maxBackends * sizeof(LOCKMETHODCTL)); /* each > > shouldn't be: > > size += maxBackends * MAXALIGN(sizeof(PROC)); /* each MyProc */ > size += maxBackends * MAXALIGN(sizeof(LOCKMETHODCTL)); /* each Yes, you are correct. The bottom one is better. > > 2) in utils/hash/dynahash.c:hash_search(): > > Assert(saveState.currElem && !(saveState.currElem = 0)); > > Does anybody know what it is for? No idea. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Tatsuo Ishii <t-ishii@sra.co.jp> writes: > I have found a few questionable codings. I'm not sure if it really > hurts anything. Suggestions are welcome. > 1) in storage/lmgr/lock.c: LockShmemSize() > size += MAXALIGN(maxBackends * sizeof(PROC)); /* each MyProc */ > size += MAXALIGN(maxBackends * sizeof(LOCKMETHODCTL)); /* each > shouldn't be: > size += maxBackends * MAXALIGN(sizeof(PROC)); /* each MyProc */ > size += maxBackends * MAXALIGN(sizeof(LOCKMETHODCTL)); /* each Probably, but I'm not sure it really makes any difference. We add on 10% or so slop after we've finished adding up all these numbers, anyway ;-) > 2) in utils/hash/dynahash.c:hash_search(): > Assert(saveState.currElem && !(saveState.currElem = 0)); > Does anybody know what it is for? That's part of that horribly ugly, non-reentrant HASH_REMOVE_SAVED interface, isn't it? I have a to-do item to rip that code out and replace it with a more reasonable design ... in the meantime, I don't think it much matters whether the Assert could be tightened up ... regards, tom lane