Hi Aleksander,

On 20/04/2021 11:30, Aleksander Alekseev wrote:
Hi Daniel,

It's my first go at submitting a patch -- it works as far as I can tell,
but I suspect there will probably still be stuff to fix before it's
ready to use!

You are doing great :)

Thanks for the encouragement!

There are several other things worth checking:
0. Always run `make distclean` before following steps
1. Make sure `make -j4 world && make -j4 check-world` passes
2. Make sure `make install-world` and `make installcheck-world` passes
3. Since you are changing the documentation it's worth checking that
it displays properly. The documentation is in the
$(PGINSTALL)/share/doc/postgresql/html directory

Several years ago I published some scripts that simplify all this a
little: https://github.com/afiskon/pgscripts, especially step 3. They
may require some modifications for your OS of choice. Please read
https://wiki.postgresql.org/wiki/Submitting_a_Patch for more
information.

Thanks for the advice (and the script repository).

One thing this has identified is an implicit declaration error on the gss_krb5_ccache_name call (the code was still working so I presume it must get included at some point, although I can't see exactly where).

This can be fixed easily enough just by adding a `#include <gssapi/gssapi_krb5.h>` line to libpq-int.h, although I don't know whether this wants to be treated differently because (as far as I can tell) it's a Kerberos-specific feature rather than something which any GSSAPI service could use (hence it being in gssapi_krb5.h rather than gssapi.h) and so might end up breaking other things?

(It looks like current versions of both MIT Kerberos and Heimdal use <gssapi/gssapi.h> rather than <gssapi.h>, although Heimdal previously had all its GSSAPI functionality, including this gss_krb5_ccache_name function, in <gssapi.h>.)

Generally speaking, it also a good idea to add some test cases for
your code, although I understand why it might be a little complicated
in this particular case. Maybe you could at least tell us how it can
be checked manually that this code actually does what is supposed to?

Something like the following code hopefully demonstrates how it's supposed to work:

const char *conninfo = "dbname='test' user='test' host='krb.local' port='5432' 
ccache_name='/home/user/test/krb5cc_1000'";
PGconn *conn;

conn = PQconnectdb(conninfo);

if(PQstatus(conn) != CONNECTION_OK) {
        fprintf(stderr, "Connection to database failed: %s\n", 
PQerrorMessage(conn));
} else {
        printf("Connection succeeded\n");
}
PQfinish(conn);

Hopefully this example gives some sort of guide to its intended purpose -- the ccache_name parameter in the connection string specifies a (non-standard) location for the credential cache, which is then used by libpq to fetch data from the database via GSSAPI authentication.

Many thanks,
Daniel


Reply via email to