Re: [dynahash] do not refill the hashkey after hash_search

From: John Naylor <johncnaylorls(at)gmail(dot)com>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: Junwang Zhao <zhjwpku(at)gmail(dot)com>, John Naylor <john(dot)naylor(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [dynahash] do not refill the hashkey after hash_search
Date: 2023-10-25 05:12:58
Message-ID: CANWCAZYc-easGz5sjDxm1iXXM14xs5_NwMu4DA5gr23CMjXmwQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Oct 13, 2023 at 10:07 AM Nathan Bossart
<nathandbossart(at)gmail(dot)com> wrote:
>
> On Thu, Sep 14, 2023 at 04:28:26PM +0800, Junwang Zhao wrote:
> > Add a v2 with some change to fix warnings about unused-parameter.
> >
> > I will add this to Commit Fest.
>
> This looks reasonable to me. I've marked the commitfest entry as
> ready-for-committer. I will plan on committing it in a couple of days
> unless John has additional feedback or would like to do the honors.

(I've been offline for a few weeks, and have been catching up this week.)

I agree it's reasonable, but there are a couple small loose ends I'd
like to see addressed.

- strlcpy(hentry->name, name, sizeof(hentry->name));

This might do with a comment stating we already set the value, (we've
seen in this thread that some other code does this), but I don't feel
strongly about it.

do
{
- hash_search(uncommitted_enums, serialized++, HASH_ENTER, NULL);
+ (void) hash_search(uncommitted_enums, serialized++, HASH_ENTER, NULL);
} while (OidIsValid(*serialized));

I still consider this an unrelated and unnecessary cosmetic change.

- NotificationHash *hentry;
bool found;

- hentry = (NotificationHash *) hash_search(pendingNotifies->hashtab,
- &oldn,
- HASH_ENTER,
- &found);
+ (void) hash_search(pendingNotifies->hashtab,
+ &oldn,
+ HASH_ENTER,
+ &found);
Assert(!found);
- hentry->event = oldn;

I'd prefer just adding "Assert(hentry->event == oldn);" and declaring
hentry PG_USED_FOR_ASSERTS_ONLY.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2023-10-25 05:21:32 Re: [dynahash] do not refill the hashkey after hash_search
Previous Message Dilip Kumar 2023-10-25 05:04:15 Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock