Everything

php

PECL Package Proposal: kadm5 - REPOST

php.internals

Holger Burbach

23 years ago

Hi everyone,

I recently developed a new extension to PHP called 'kadm5'. This
extension enables you to create, modify, and delete Kerberos V principals
and policies on a remote Kerberos Administration server.

More information about Kerberos can be found at:
http://web.mit.edu/kerberos/www/

There is a new release of kadm5 with some minor changes. The sources can
be found at:

ftp://oss.gonicus.de/pub/php-kadm5/php-kadm5-0.2.2/

So far I was able to collect 3 votes from pear-dev:

+1   "Michael Bretterklieber" <mbretter@jawa.at>
+1   "Andrey Hristov" <andrey@php.net>
+1   "Yavor Shahpasov" <yavo@siava.org>

Any other comments? Still need 2 votes...
I'm looking forward to integrate this package into PECL.

Regards,
Holger

Sander Roobol

23 years ago

On Fri, May 16, 2003 at 03:25:04PM +0200, Holger Burbach wrote:

> There is a new release of kadm5 with some minor changes. The sources can
> be found at:
>
> ftp://oss.gonicus.de/pub/php-kadm5/php-kadm5-0.2.2/

Looks fine, BUT (Derick, still reading? :)

* There are some comments from the ext/skel that should (IMO) be
removed. Stuff like 'If you have INI entries ...'
* The PHP_FUNCTION()s don't have protos. Please add them.
* There are some whitespace inconsistancies here and there.

Apart from that +1.

Sander

Derick Rethans

23 years ago

On Fri, 16 May 2003, Sander Roobol wrote:

> On Fri, May 16, 2003 at 03:25:04PM +0200, Holger Burbach wrote:
> > There is a new release of kadm5 with some minor changes. The sources can
> > be found at:
> >
> > ftp://oss.gonicus.de/pub/php-kadm5/php-kadm5-0.2.2/
>
> Looks fine, BUT (Derick, still reading? :)

1. The biggest problem I see is that the extension is throwing E_ERRORs
which extensions really should not do if they do not leave *PHP* in
an unstable state (there is also no \n required at the end of the text,
and in the best case php_error_docref should be used).

2. Check our CODING STANDARDS file again :)

3. I would recommend to use this naming (As example):
kadm5_principal_create
kadm5_principal_modify

instead of:
kadm5_create_principal
kadm5_modify_principal

4. There are no prototypes for the PHP_FUNCTIONs

5. There should be atleast one test per function, if possible.

6. The package file has some unescapted entities (the & in "Cleanup in
kadm5.c: Removal of confirm_kadm5_compiled & RINIT/RSHUTDOWN" for
example)

regards,
Derick

--
"my other box is your windows PC"
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
PHP Magazine - PHP Magazine for Professionals       http://php-mag.net/
-------------------------------------------------------------------------

Holger Burbach

23 years ago

On Freitag, 16. Mai 2003 15:46, Derick Rethans wrote:

> On Fri, 16 May 2003, Sander Roobol wrote:
> > On Fri, May 16, 2003 at 03:25:04PM +0200, Holger Burbach wrote:
> > > There is a new release of kadm5 with some minor changes. The sources
> > > can be found at:
> > >
> > > ftp://oss.gonicus.de/pub/php-kadm5/php-kadm5-0.2.2/
> >
> > Looks fine, BUT (Derick, still reading? :)
>
> 1. The biggest problem I see is that the extension is throwing E_ERRORs
>    which extensions really should not do if they do not leave *PHP* in
>    an unstable state (there is also no \n required at the end of the text,
>    and in the best case php_error_docref should be used).

So throwing E_WARNING would be better?

> 2. Check our CODING STANDARDS file again :)

Okay, I'm working on that.

> 3. I would recommend to use this naming (As example):
>    kadm5_principal_create
>    kadm5_principal_modify
>
>    instead of:
>    kadm5_create_principal
>    kadm5_modify_principal

Sounds reasonable.

> 4. There are no prototypes for the PHP_FUNCTIONs

Okay, I'm working on that.

> 5. There should be atleast one test per function, if possible.

Hmm, to test the functions I have to connect to an Kerberos V administration
server...

> 6. The package file has some unescapted entities (the & in "Cleanup in
>    kadm5.c: Removal of confirm_kadm5_compiled & RINIT/RSHUTDOWN" for
>    example)

Well, it seems that I have some things to do...

Regards,
Holger

Derick Rethans

23 years ago

Hey Holger,

On Fri, 16 May 2003, Holger Burbach wrote:

> > 1. The biggest problem I see is that the extension is throwing E_ERRORs
> >    which extensions really should not do if they do not leave *PHP* in
> >    an unstable state (there is also no \n required at the end of the text,
> >    and in the best case php_error_docref should be used).
>
> So throwing E_WARNING would be better?

Definitely. (After an E_WARNING just return FALSE (or NULL)).

> > 2. Check our CODING STANDARDS file again :)
>
> Okay, I'm working on that.
>
> > 3. I would recommend to use this naming (As example):
> >    kadm5_principal_create
> >    kadm5_principal_modify
> >
> >    instead of:
> >    kadm5_create_principal
> >    kadm5_modify_principal
>
> Sounds reasonable.
>
> > 4. There are no prototypes for the PHP_FUNCTIONs
>
> Okay, I'm working on that.
>
> > 5. There should be atleast one test per function, if possible.
>
> Hmm, to test the functions I have to connect to an Kerberos V administration
> server...

Yeah, I know :) But tests also can work as examples so there is
something to gain there. The DBX (ext/dbx) guys figured out something
clever to handle cases where the DB server was not available, perhaps
you can have a look on how they did it.


good luck,
Derick

--
"my other box is your windows PC"
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
International PHP Magazine                          http://php-mag.net/
-------------------------------------------------------------------------

Holger Burbach

23 years ago

Hi Derick,

can you please have another look at my PECL module kadm5? I think that I have
fixed all the issues you mentioned.

Well, hoping to get the final vote "+1" from you. ;-)
Thanks in advance.

Bye
Holger

On Friday, 16. Mai 2003 15:46, Derick Rethans wrote:

> On Fri, 16 May 2003, Sander Roobol wrote:
> > On Fri, May 16, 2003 at 03:25:04PM +0200, Holger Burbach wrote:
> > > There is a new release of kadm5 with some minor changes. The sources
> > > can be found at:
> > >
> > > ftp://oss.gonicus.de/pub/php-kadm5/php-kadm5-0.2.2/
> >
> > Looks fine, BUT (Derick, still reading? :)
>
> 1. The biggest problem I see is that the extension is throwing E_ERRORs
>    which extensions really should not do if they do not leave *PHP* in
>    an unstable state (there is also no \n required at the end of the text,
>    and in the best case php_error_docref should be used).

Fixed.

> 2. Check our CODING STANDARDS file again :)

Fixed. At least I hope so ;-)

> 3. I would recommend to use this naming (As example):
>    kadm5_principal_create
>    kadm5_principal_modify
>
>    instead of:
>    kadm5_create_principal
>    kadm5_modify_principal

Well, I see your point, but changing the function names gets quite confusing
for someone who is familiar with the kadm5 C API. I would prefer to keep them
the way they are.

> 4. There are no prototypes for the PHP_FUNCTIONs

Fixed.

> 5. There should be atleast one test per function, if possible.

If someone has a clever idea, how to implement these test without accessing a
real Kerberos server, I would love to do it.

> 6. The package file has some unescapted entities (the & in "Cleanup in
>    kadm5.c: Removal of confirm_kadm5_compiled & RINIT/RSHUTDOWN" for
>    example)

Fixed.

Holger Burbach

23 years ago

On Donnerstag, 22. Mai 2003 15:04, Holger Burbach wrote:

> Hi Derick,
>
> can you please have another look at my PECL module kadm5? I think that I
> have fixed all the issues you mentioned.

Sorry, the sources can be found at:

ftp://oss.gonicus.de/pub/php/php-kadm5/php-kadm5-0.2.3/

Derick Rethans

23 years ago

On Thu, 22 May 2003, Holger Burbach wrote:

> > can you please have another look at my PECL module kadm5? I think that I
> > have fixed all the issues you mentioned.
>
> Sorry, the sources can be found at:
>
> ftp://oss.gonicus.de/pub/php/php-kadm5/php-kadm5-0.2.3/

I'll have a look when I get home.

Derick

--
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
International PHP Magazine                          http://php-mag.net/
-------------------------------------------------------------------------