This patch finally allows us to define the type for basic options. It uses a union to store different types, and an enum to list the allowed types. It also provides for helper functions that always check the type to safely retrieve data, or scream loudly if we messed up.
This allowed me to remove all but 1 special option that was a duplicate made only to avoid converting from string to int every time we needed to get an option. The only remaining special case is the schema type, because that is a special string to special number transaltion. But these kind of options (another is tls_reqcert) are rare enough that we can avoid trying to make special handlers for them too.
Currently there are 4 types supported: string, blob, number, boolean. String has 2 subtypes, const and non const, but they are not enforced (you can assign a const string and then re-read it as non-const). But this is not really a problem, as values are always copied in the init functions, furthermore, although you can get values as non-const they should always be regarded a immutable strings as the value returned is the actual string saved in the option, so changing it, means changing the configuration.
I think we can later take this code and make it generic in the provider backend code, so that all backends can use it. It should be easy enough to do.
Simo.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 09/13/2009 10:38 AM, Simo Sorce wrote:
This patch finally allows us to define the type for basic options. It uses a union to store different types, and an enum to list the allowed types. It also provides for helper functions that always check the type to safely retrieve data, or scream loudly if we messed up.
This allowed me to remove all but 1 special option that was a duplicate made only to avoid converting from string to int every time we needed to get an option. The only remaining special case is the schema type, because that is a special string to special number transaltion. But these kind of options (another is tls_reqcert) are rare enough that we can avoid trying to make special handlers for them too.
Currently there are 4 types supported: string, blob, number, boolean. String has 2 subtypes, const and non const, but they are not enforced (you can assign a const string and then re-read it as non-const). But this is not really a problem, as values are always copied in the init functions, furthermore, although you can get values as non-const they should always be regarded a immutable strings as the value returned is the actual string saved in the option, so changing it, means changing the configuration.
I think we can later take this code and make it generic in the provider backend code, so that all backends can use it. It should be easy enough to do.
Simo.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Nack. Generates a segfault running "getent passwd user@LDAP"
- -- Stephen Gallagher RHCE 804006346421761
Looking to carve out IT costs? www.redhat.com/carveoutcosts/
On Mon, Sep 14, 2009 at 08:03:14AM -0400, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 09/13/2009 10:38 AM, Simo Sorce wrote:
This patch finally allows us to define the type for basic options. It uses a union to store different types, and an enum to list the allowed types. It also provides for helper functions that always check the type to safely retrieve data, or scream loudly if we messed up.
This allowed me to remove all but 1 special option that was a duplicate made only to avoid converting from string to int every time we needed to get an option. The only remaining special case is the schema type, because that is a special string to special number transaltion. But these kind of options (another is tls_reqcert) are rare enough that we can avoid trying to make special handlers for them too.
Currently there are 4 types supported: string, blob, number, boolean. String has 2 subtypes, const and non const, but they are not enforced (you can assign a const string and then re-read it as non-const). But this is not really a problem, as values are always copied in the init functions, furthermore, although you can get values as non-const they should always be regarded a immutable strings as the value returned is the actual string saved in the option, so changing it, means changing the configuration.
I think we can later take this code and make it generic in the provider backend code, so that all backends can use it. It should be easy enough to do.
Simo.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Nack. Generates a segfault running "getent passwd user@LDAP"
I cannot reproduce this with my setup, but
providers/ldap/ldap_id.c:982: warning: 'ldap_id_cleanup_done' defined but not used
and there are some typos in the title and description of the patch.
bye, Sumit
On Mon, 2009-09-14 at 14:34 +0200, Sumit Bose wrote:
Nack. Generates a segfault running "getent passwd user@LDAP"
I cannot reproduce this with my setup, but
providers/ldap/ldap_id.c:982: warning: 'ldap_id_cleanup_done' defined but not used
and there are some typos in the title and description of the patch.
Ok thanks to Steve I could jump on a machine where this was reproducible (different build flags ?). It was a bug in the code that initialized the blob type in case the option was not specified.
the warning was instead a leftover of another patch I am working on, it shouldn't happen now.
New patch attached.
Simo.
On Mon, Sep 14, 2009 at 11:32:40AM -0400, Simo Sorce wrote:
On Mon, 2009-09-14 at 14:34 +0200, Sumit Bose wrote:
Nack. Generates a segfault running "getent passwd user@LDAP"
I cannot reproduce this with my setup, but
providers/ldap/ldap_id.c:982: warning: 'ldap_id_cleanup_done' defined but not used
and there are some typos in the title and description of the patch.
Ok thanks to Steve I could jump on a machine where this was reproducible (different build flags ?). It was a bug in the code that initialized the blob type in case the option was not specified.
the warning was instead a leftover of another patch I am working on, it shouldn't happen now.
New patch attached.
It's still working for me :-). It would be nice to use this scheme in other places soon.
ACK
bye, Sumit
On Mon, 2009-09-14 at 18:21 +0200, Sumit Bose wrote:
On Mon, Sep 14, 2009 at 11:32:40AM -0400, Simo Sorce wrote:
On Mon, 2009-09-14 at 14:34 +0200, Sumit Bose wrote:
Nack. Generates a segfault running "getent passwd user@LDAP"
I cannot reproduce this with my setup, but
providers/ldap/ldap_id.c:982: warning: 'ldap_id_cleanup_done' defined but not used
and there are some typos in the title and description of the patch.
Ok thanks to Steve I could jump on a machine where this was reproducible (different build flags ?). It was a bug in the code that initialized the blob type in case the option was not specified.
the warning was instead a leftover of another patch I am working on, it shouldn't happen now.
New patch attached.
It's still working for me :-). It would be nice to use this scheme in other places soon.
ACK
pushed
Simo.
sssd-devel@lists.fedorahosted.org
-
Simo Sorce -
Stephen Gallagher -
Sumit Bose