Re: Patches to KADM5
| From: | Johannes Schlüter | Date: | Mon, 20 Feb 2012 22:34:16 +0000 |
| Subject: | Re: Patches to KADM5 | ||
| References: | 1 2 3 | Groups: | php.pecl.dev |
| Request: | Send a blank email to pecl-dev+get-9158@lists.php.net to get a copy of this message | ||
Hi,
On Tue, 2012-02-21 at 09:20 +1100, Edward Murrell wrote:
> Hi, as requested, attached is the patch. This compiled and installed
> happily with the following commands, which I'm led to believe is the
> expected behaviour.
I went very quickly over it very few small comments:
>
> diff -u kadm5-0.2.3//config.m4 kadm5-0.2.4//config.m4
> --- kadm5-0.2.3//config.m4 2003-05-10 00:28:45.000000000 +1000
> +++ kadm5-0.2.4//config.m4 2012-02-21 09:07:04.898454002 +1100
> @@ -59,7 +59,6 @@
> PHP_ADD_LIBRARY_WITH_PATH(gssrpc, /usr/lib, KADM5_SHARED_LIBADD)
> PHP_ADD_LIBRARY_WITH_PATH(krb5, /usr/lib, KADM5_SHARED_LIBADD)
> PHP_ADD_LIBRARY_WITH_PATH(k5crypto, /usr/lib, KADM5_SHARED_LIBADD)
> - PHP_ADD_LIBRARY_WITH_PATH(dyn, /usr/lib, KADM5_SHARED_LIBADD)
Why is that lib not needed anymore? Is that version dependent? Maybe it
needs a version check?
> PHP_EXTENSION(kadm5, $ext_shared)
> fi
> diff -u kadm5-0.2.3//CREDITS kadm5-0.2.4//CREDITS
> --- kadm5-0.2.3//CREDITS 2003-05-08 17:48:51.000000000 +1000
> +++ kadm5-0.2.4//CREDITS 2012-02-21 09:07:04.898454002 +1100
> @@ -1,2 +1,3 @@
> kadm5
> Holger Burbach
> +Edward Murrell
Those should be in one line:
-Holger Burbach
+Holger Burbach, Edward Murrell
I don't think it's a real issue but in the theoretical case of it being
bundled with PHP this would be required for generating the mein PHP
credits. This might also be required for (in this case irrelevant)
Windows builds to fill the properties of the extension's dll.
> @@ -436,6 +436,7 @@
> php_error(E_WARNING, "Multiple values for single or
> folded enctype. (KADM5_SETKEY_DUP_ENCTYPES)");
Changing this to php_error_docref() gies an a bit nicer error message.
> break;
> default:
> + break;
> }
> }
> /* }}} */
> @@ -450,13 +451,15 @@
> kadm5_config_params params;
> void *handle = NULL;
> kadm5_ret_t rc;
> + krb5_error_code rv;
> + krb5_context ctx;
> + char **db_args = NULL;
>
> memset((char *) ¶ms, 0, sizeof(params));
>
> - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ssss",
> -
> &admin_server, &admin_server_len,
> - &realm,
> &realm_len, &princstr, &princstr_len,
> - &password,
> &password_len) == FAILURE) {
> + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ssss",
> + &admin_server, &admin_server_len, &realm, &realm_len,
> + &princstr, &princstr_len, &password, &password_len) ==
> FAILURE) {
> WRONG_PARAM_COUNT;
This should be a single return; else the user gets two error
messages.
> }
>
> @@ -465,13 +468,22 @@
> params.admin_server = admin_server;
> params.mask |= KADM5_CONFIG_ADMIN_SERVER;
>
> - rc = kadm5_init_with_password(princstr,
> -
> password,
> -
> KADM5_ADMIN_SERVICE,
> -
> ¶ms,
> -
> KADM5_STRUCT_VERSION,
> -
> KADM5_API_VERSION_2,
> -
> &handle);
> + rv = krb5_init_context(&ctx);
> + if (rv) {
> + php_error(E_WARNING, "Error while initializing krb5
> library");
php_error_docref(), as mentioned above.
> + RETURN_FALSE;
> + }
> +
> + rc = kadm5_init_with_password(
> + ctx,
> + princstr,
> + password,
> + KADM5_CHANGEPW_SERVICE,
> + ¶ms,
> + KADM5_STRUCT_VERSION,
> + KADM5_API_VERSION_3,
> + db_args,
> + &handle);
Will this work with all versions or should there be a check in config.m4
to disallow old versions of the lib?
As said: All just tiny things. Nothing big.
Any comments from anybody else?
johannes