Claudio Natoli <[EMAIL PROTECTED]> writes:
>> You should probably check the return value of unlink().
>
> No. This isn't necessary (and what action would it take in any
> case?).

It should write a log message. I'm not sure why this /shouldn't/ be
done: if an operation fails, we should log that failure. This is
standard practise.

>> Doesn't this function still acquire ShmemIndexLock? (i.e. why was
>> this comment changed?)
>
> AFAICS this is just whitespace differences.

Read it again. Here's the whole diff hunk:

*** 320,340 ****
        strncpy(item.key, name, SHMEM_INDEX_KEYSIZE);
        item.location = BAD_LOCATION;
  
!       LWLockAcquire(ShmemIndexLock, LW_EXCLUSIVE);
  
        if (!ShmemIndex)
        {
                /*
                 * If the shmem index doesn't exist, we are bootstrapping: we must
                 * be trying to init the shmem index itself.
                 *
!                * Notice that the ShmemIndexLock is held until the shmem index has
                 * been completely initialized.
                 */
                Assert(strcmp(name, "ShmemIndex") == 0);
                Assert(ShmemBootstrap);
                *foundPtr = FALSE;
!               return ShmemAlloc(size);
        }
  
        /* look it up in the shmem index */
--- 335,367 ----
        strncpy(item.key, name, SHMEM_INDEX_KEYSIZE);
        item.location = BAD_LOCATION;
  
!       SpinLockAcquire(ShmemIndexLock);
  
        if (!ShmemIndex)
        {
+               if (IsUnderPostmaster)
+               {
+                       /* Must be initializing a (non-standalone) backend */
+                       Assert(strcmp(name, "ShmemIndex") == 0);
+                       Assert(ShmemBootstrap);
+                       Assert(ShmemIndexAlloc);
+                       *foundPtr = TRUE;
+               }
+               else
+               {
                        /*
                         * If the shmem index doesn't exist, we are bootstrapping: we 
must
                         * be trying to init the shmem index itself.
                         *
!                        * Notice that the ShmemLock is held until the shmem index has
                         * been completely initialized.
                         */
                        Assert(strcmp(name, "ShmemIndex") == 0);
                        Assert(ShmemBootstrap);
                        *foundPtr = FALSE;
!                       ShmemIndexAlloc = ShmemAlloc(size);
!               }
!               return ShmemIndexAlloc;
        }

The code acquires ShmemIndexLock, performs some computations, and then
notes that "ShmemLock is held" in a comment before returning. ISTM
that is plainly wrong.

> [ the only other suggested changes are ] stylistic/cosmetic and
> effect the EXEC_BACKEND code only.

Yeah, my apologies for nitpicking...

-Neil


---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]

Reply via email to