Re: [PATCH 1/2 v3] [libpq] rework sigpipe-handling macros

Hi Tom,

> That code is not broken as it stands, and doesn't appear to really
> gain anything from the proposed change.  Why should we risk any
> portability questions when the code isn't going to get either simpler
> or shorter?

OK, after attempting a macro version of this, we end up with:

#define DECLARE_SIGPIPE_INFO(var) struct sigpipe_info var;

#define DISABLE_SIGPIPE(info, conn) \((SIGPIPE_MASKED(conn)) ? 0 : \	((info)->got_epipe = false, \	pg_block_sigpipe(&(info)->oldsigmask, &(info)->sigpipe_pending)) < 0)

- kinda ugly, uses the ?: and , operators to return the result of 
pg_block_sigpipe. We could avoid the comma with a block though.

If we want to keep the current 'failaction' style of macro:

#define DISABLE_SIGPIPE(info, conn, failaction) \  do { \	if (!SIGPIPE_MASKED(conn)) { \		(info)->got_epipe = false; \		if (pg_block_sigpipe(&(info)->oldsigmask, \				&(info)->sigpipe_pending)) < 0) { \			failaction; \		} \	} \} while (0)

We could ditch struct sigpipe info, but that means we need to declare 
three separate vars in DECLARE_SIGPIPE_INFO() instead of the one, and it 
doesn't clean up the macro code much. I'd rather reduce the amount of 
stuff that we declare "behind the caller's back".

Compared to the static-function version:

static inline int disable_sigpipe(PGconn *conn, struct sigpipe_info 
*info)
{if (sigpipe_masked(conn))	return 0;
info->got_epipe = false;return pq_block_sigpipe(&info->oldsigmask, &info->sigpipe_pending) < 0;
}

Personally, I think the static functions are a lot cleaner, and don't 
think we lose any portability from using these (since inline is #define-
ed out on compilers that don't support it). On non-inlining compilers, 
we do gain an extra function call, but we're about to enter the kernel 
anyway, so this will probably be lost in the noise (especially if we 
save the sigpipe-masking syscalls).

But in the end, it's not up to me - do you still prefer the macro 
approach?

Cheers,


Jeremy