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

Tue, 14 Dec 1999 17:54:54 +0900
Tatsuo Ishii <t-ishii@sra.co.jp>
Questionable codes
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

Tue, 14 Dec 1999 11:23:24 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] 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

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, Pennsylvania 19026

Tue, 14 Dec 1999 11:25:10 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [HACKERS] Questionable codes