public inbox for [email protected]  

help / color / mirror / Atom feed

From: Neil Conway <[email protected]>
To: Nathan Bossart <[email protected]>
Cc: [email protected]
Subject: Re: small pg_dump code cleanup
Date: Wed, 5 Jun 2024 12:22:03 -0400
Message-ID: <CAOW5sYZY3sdSxzaRwQCMMNtsZ_1jN+AFzB82k58==Vome-G11w@mail.gmail.com> (raw)
In-Reply-To: <ZmCAtVaOrHpf31PJ@nathan>
References: <ZmCAtVaOrHpf31PJ@nathan>

On Wed, Jun 5, 2024 at 11:14 AM Nathan Bossart <[email protected]>
wrote:

>  In fact, many of the functions in this area don't actually need to

return anything, so we can trim some code and hopefully reduce confusion a
> bit.  Patch attached.
>

Nice cleanup! Two minor comments:

(1) Names like `getXXX` for these functions suggest to me that they return
a value, rather than side-effecting. I realize some variants continue to
return a value, but the majority no longer do. Perhaps a name like
lookupXXX() or readXXX() would be clearer?

(2) These functions malloc() a single ntups * sizeof(struct) allocation and
then index into it to fill-in each struct before entering it into the hash
table. It might be more straightforward to just malloc each individual
struct.

Neil

view thread (7+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: small pg_dump code cleanup
  In-Reply-To: <CAOW5sYZY3sdSxzaRwQCMMNtsZ_1jN+AFzB82k58==Vome-G11w@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox