Re: Password identifiers, protocol aging and SCRAM protocol

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, David Steele <david(at)pgmasters(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Fetter <david(at)fetter(dot)org>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Julian Markwort <julian(dot)markwort(at)uni-muenster(dot)de>, Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Valery Popov <v(dot)popov(at)postgrespro(dot)ru>
Subject: Re: Password identifiers, protocol aging and SCRAM protocol
Date: 2016-11-15 18:40:34
Message-ID: CA+TgmoYAqsiB3SAzmz_bBhQ6k31Osp=DnBX-2n-FP37ShS8QNA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 4, 2016 at 11:58 AM, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> The organization of these patches makes sense to me.
>
> On 10/20/16 1:14 AM, Michael Paquier wrote:
>> - 0001, moving all the SHA2 functions to src/common/ and introducing a
>> PG-like interface. No actual changes here.
>
> That's probably alright, although the patch contains a lot more changes
> than I would imagine for a simple file move. I'll still have to review
> that in detail.

Even with git diff -M, reviewing 0001 is very difficult. It does
things that are considerably in excess of what is needed to move these
files from point A to point B, such as:

- Renaming static functions to have a "pg" prefix.
- Changing the order of the functions in the file.
- Renaming an argument called "context" to "cxt".

I think that is a bad plan. I think we should insist that 0001
content itself with a minimal move of the files changing no more than
is absolutely necessary. If refactoring is needed, those changes can
be submitted separately, which will be much easier to review. My
preliminary judgement is that most of this change is pointless and
should be reverted.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-11-15 18:43:00 Re: Snapshot too old logging
Previous Message Brad DeJong 2016-11-15 18:33:55 Re: Snapshot too old logging