On 11/07/2014 05:40 PM, Nathaniel McCallum wrote:
On Fri, 2014-11-07 at 15:02 +0100, thierry bordaz wrote:
On 11/04/2014 05:17 PM, Nathaniel McCallum wrote:

On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote:
On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote:
On 10/29/2014 10:37 AM, Martin Kosek wrote:
On 10/28/2014 09:59 PM, Nathaniel McCallum wrote:
On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:
This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.


NOTE: There is one known issue with this patch which I don't know how to
solve. This patch changes the schema in install/share/60ipaconfig.ldif.
On an upgrade, all of the new attributeTypes appear correctly. However,
the modifications to the pre-existing objectClass do not show up on the
server. What am I doing wrong?

After modifying ipaGuiConfig manually, everything in this patch works
just fine.
This new version takes into account the new (proper) OIDs and attribute
Thanks Nathaniel!

The above known issue still remains.
Petr3, any idea what could have gone wrong? ObjectClass MAY list extension
should work just fine, AFAIK.
You added a blank line to the LDIF file. This is an entry separator, so
the objectClasses after the blank line don't belong to cn=schema, so
they aren't considered in the update.
Without the blank line it works fine.
Thanks for the catch!

Here is a version without the blank line.
I forgot to remove the old steps defines. This patch performs this

Freeipa-devel mailing list
Hello Nathaniel,

         Few comments on the review:
               * in authcfg
                       * in string_to_types, I would prefer you set the
                         last element of 'map' to { NULL, 0 }.
Why? What I have is perfectly legal ISO C and is exactly the equivalent
of your code. In a structure initializer, undefined structure elements
are zero'd.

                       * in entry_to_window, you may declare the
                         'defaults' array as 'static const'

                       * Would use define for
                         "ipaHOTPSyncWindow","ipaTOTPSyncWindow" that
                         are present multiple times

                       * suffix_to_config: cfg is set (and returned)
                         calling entry_to_config(entry). Now the
                         entry_to_config returns a structure on the
                         stack so it is not valid to access outside of
                         the entry_to_config
Nope. We are not passing by reference but by copy. This is perfectly
valid C.

                       *  authcfg_fini free the configs. config->cfg
                         should have been allocated and must be freed
                         (be care that configs->cfg may contains
Nope. The config->cfg structure is allocated and freed when config is.
This is assignment by copy not by reference.

                       * authcfg_get_auth_types:322 should it return
                         'gbl' or AUTHCFG_AUTH_TYPE_PASSWORD
If both the global and per-user auth type is unset, the default is
AUTHCFG_AUTH_TYPE_PASSWORD. We special case this here so that we don't
have to special case it everywhere else in the code. The code is correct
as stands.

                       * authcfg_get_auth_window/authcfg_get_sync_window 
returns a window structure that is on the stack. It is not valid outside of 
those functions
Nope. Structure return by copy is perfectly legal ISO C.
Hi Nathaniel,

   You are right, I am not use to structure assignment and all these
   assignments are valid.
   Sorry for the noise. The patch is fine for me.


Freeipa-devel mailing list

Reply via email to