Re: [PATCH v1] GSSAPI encryption support

On Wed, Mar 30, 2016 at 1:01 PM, Robbie Harwood <rharwood(at)redhat(dot)com> wrote:
> A new version of my GSSAPI encryption patchset is available, both in
> this email and on my github:
> https://github.com/frozencemetery/postgres/tree/feature/gssencrypt9
>
> This version is intended to address David's review suggestions:
>
> - The only code change (not counting SGML and comments) is that I've
> resolved the pre-existing "XXX: Should we loop and read all messages?"
> comment in the affirmative by... looping and reading all messages in
> pg_GSS_error. Though commented on as part of the first patch, this is
> bundled with the rest of the auth cleanup since the first patch is
> code motion only.
>
> - Several changes to comments, documentation rewordings, and whitespace
> additions. I look forward to finding out what needs even more of the
> same treatment. Of all the changesets I've posted thus far, this
> might be the one for which it makes the most sense to see what changed
> by diffing from the previous changeset.

Thanks for the new versions. For now I have been having a look at only 0001.

The first thing I have noticed is that 0001 breaks the Windows build
using Visual Studio. When building without GSSAPI, fe-gssapi-common.c
and be-gssapi-common.c should be removed from the list of files used
so that's simple enough to fix. Now when GSSAPI build is enabled,
there are some declaration conflicts with your patch, leading to many
compilation errors. This took me some time to figure out but the cause
is this diff in be-gssapi-common.c:
+#include "libpq/be-gssapi-common.h"
+
+#include "postgres.h"

postgres.h should be declared *before* be-gssapi-common.h. As I
suggested the refactoring and I guess you don't have a Windows
environment at hand, attached is a patch that fixes the build with and
without gssapi for Visual Studio, that can be applied on top of your
0001. Feel free to rebase using it.

Note for others: I had as well to patch the MSVC scripts because in
the newest installations of Kerberos:
- inc/ does not exist anymore, include/ does
- The current VS scripts ignore completely x64 builds, the libraries
linked to are for x86 unconditionally.
I am not sure if there are *any* people building the code with
Kerberos on Windows except I, but as things stand those scripts are
rather broken in my view.

Now looking at 0002 and 0003...
--
Michael