openldap-devel

openldap-devel@openldap.org

1418 discussions

commit sizes, coding conventions, and back-ldif
by Hallvard B Furuseth 23 Mar '08

23 Mar '08

My patch to back-ldif/ldif.c for ITS#5408 is larger than the ldif.c file itself, which has left me wondering what to do with monster pathces: Should they be split up in stepwise commits so it's easier to see the logic of each change? When reading patches, I vastly prefer them split up that way, but I don't know how much that is worth in the CVS tree. E.g. first a commit which preserves the program logic including bugs and just moves code around, then one with a bugfix which now can be small and easy to read, then one for the next bug, and so on. Besides being more work, one problem with that approach is that one can make an error in an poorly tested intermediate version - since that version was never intended to be used anyway. Also I don't know what impact it has on CVS merging. Also, what's the thereshold for when it makes sense to reformat and rearrange code just for the sake of readability? back-ldif ihas no consistent coding style. Normally it's "don't reformat, it makes CVS merges harder". In this case it was easy with some of it - I touch more than half of the lines anyway, so I could just as well reformat those and a few more to something closer to the OpenLDAP conventions (to the degree I understand them). But maybe in this case I should just as well reformat it all, and maybe also it's a good time time to split back-ldif up in 9 files of ~100-300 lines instead of one of ~1500 lines. (Regarding CVS merges, the HEAD and RE24 versions are equal, but RE23 differs.) Another extreme example was a recent change to a big chunk of code, where the only change was to wrap the code in a new if-test and indent it. It would have been nice with a a comment in the commit, or a 1-line commit + a cosmetic commit. Regarding formatting, when I do it I'd prefer to do it right - but I've never quite figured out what the preferred conventions are. We've had a style threads before but I don't remember if any of them got very far. So I thought I'd list some rules and confusions of my own that I've noticed. * Generous whitespace: - Around parens in control statements - "if ( x )" etc. - Inside non-empty parens in function calls, sometimes sizeof, sometimes inside []. - Usually around binary operators. However often but not always whitespace between two parens are omitted - e.g. "foo( bar( baz ))". Is there some preferred guideline about that? How about whitespace around parens that are for grouping? E.g. "if ( (ch = getc(f)) != EOF )". * Similarly generous use of {} for if statements etc, usually used even when enclosing just a single statement. * Indent 1 tab for continuation lines with the same paren level, e.g. x = foo( bar, baz ); y = one ? two : three; BTW, emacs will format this the OpenLDAP way: wups( foo, bar ); but if there is anything after the '(' it aligns bar with it: wups( foo, bar ); OpenLDAP code often does put arguments after the '(' but to my eyes it looks easier to read without that, in addition to making emacs happy. Complex multi-line statements require more creativity, but I can't tell what, if any, ideas lie behind their indentation (beyond trying to avoid such statements). Don't know if e.g. this is OpenLDAPpy: if ( (one( two, three ) && (four || (five && six))) ) * Tab-width = 4 columns. Makes a difference for when to break lines (keeping line length < 80), how to align comments after code on a line, and to align declarations and multi-line statements. It doesn't hurt to try to keep code nice-looking with tab width 8 too. (E.g. comments above statements instead of aligned to the right, and often do not align variables in declarations.) * Indent with tabs, not spaces. * Usually align comments/decls to the right of code with tabs too, but not always. (If we used space to align after code, it would stay aligned regardless of tab-width.) * Function names at first column in function definitions. -- Hallvard

2 3

printing Sleepycat version always
by Aaron Richton 22 Mar '08

22 Mar '08

[purposefully on -devel for discussion] Any opinions on: --- init.c~ 2008-03-12 10:02:44.000000000 -0400 +++ init.c 2008-03-12 10:03:16.000000000 -0400 @@ -712,7 +712,7 @@ return -1; } - Debug( LDAP_DEBUG_TRACE, LDAP_XSTRING(bdb_back_initialize) + Debug( LDAP_DEBUG_ANY, LDAP_XSTRING(bdb_back_initialize) ": %s\n", version, 0, 0 ); } I'm definitely one for 'quiet' software, but knowing the version of libdb is a really nice detail when assisting users of OpenLDAP. This would make it that much easier to have them find it. I'd argue there's a bit of precedent: main.c: Debug( LDAP_DEBUG_ANY, "%s", Versionstr, 0, 0 ); The negative, of course, is the noise. But what's ~60 bytes in this day and age?

5 7

glue overlay man page needed?
by Gavin Henry 21 Mar '08

21 Mar '08

Hi all, There is no configuration for the glue overlay other than "overlay glue" in the right place. So would there be a point having a man page? The only mention of the glue overlay is in the slapd.conf man page. Thanks, Gavin.

1 0

Re: slapd new backend
by Malcolm Cowen 20 Mar '08

20 Mar '08

"My first question is why a new backend? Why are you doing it if you've never seen OpenLDAP code before?" They want it to link to a product of theirs, on a particular product and platform where we do have significant experience. (I can't give details because of commercial confidenciality - although Client does understand the open source nature of OpenLDAP) "[Read] the source" - Of course Malcolm Cowen, Software Developer, Cowen Software Ltd: 21-23 Bristol Ave, Levenshulme, Manchester, GB- M19 3NU, UK +44 (0) 161 225 4674 (office) +44 (0) 7973 950 597 (mobile) Registered in England No: 1828889, VAT Reg 403 4867 60 www.cowensw.co.uk, www.cowensw.com, email: mct(a)cowensw.co.uk

2 1

SummerOfCode suggestions
by Howard Chu 19 Mar '08

19 Mar '08

I recall we had a few ideas last year, but they came up a bit late. Anyone still have some wish list items for this year? -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

9 18

Re: commit: openldap-guide/admin access-control.sdf set-following-references.png set-memberUid.png set-recursivegroup.png Makefile aspell.en.pws master.sdf slapdconf2.sdf slapdconfig.sdf
by Pierangelo Masarati 14 Mar '08

14 Mar '08

ghenry(a)OpenLDAP.org wrote: > access-control.sdf NONE -> 1.1 I see an error here: > <access directive> ::= access to <what> > [by <who> [<access>] [<control>] ]+ > <what> ::= * | > [dn[.<basic-style>]=<regex> | dn.<scope-style>=<DN>] If no [.<basic-style>] is given, the default is <DN>; this rule should look like > [dn[.<scope-style>]=<DN> | dn.regex=<regex>] and basic style be changed/removed accordingly. p. Ing. Pierangelo Masarati OpenLDAP Core Team SysNet s.r.l. via Dossi, 8 - 27100 Pavia - ITALIA http://www.sys-net.it --------------------------------------- Office: +39 02 23998309 Mobile: +39 333 4963172 Email: pierangelo.masarati(a)sys-net.it ---------------------------------------

2 2

Re: commit: ldap/servers/slapd backend.c operation.c proto-slap.h
by Howard Chu 13 Mar '08

13 Mar '08

hyc(a)OpenLDAP.org wrote: > Update of /repo/OpenLDAP/pkg/ldap/servers/slapd > > Modified Files: > backend.c 1.394 -> 1.395 > operation.c 1.85 -> 1.86 > proto-slap.h 1.751 -> 1.752 > > Log Message: > ITS#5419 introduce OpExtra / op->o_extra to supersede op->o_private This code should probably use <ldap_queue.h> instead of the current approach. > > CVS Web URLs: > http://www.openldap.org/devel/cvsweb.cgi/servers/slapd/ > http://www.openldap.org/devel/cvsweb.cgi/servers/slapd/backend.c > http://www.openldap.org/devel/cvsweb.cgi/servers/slapd/operation.c > http://www.openldap.org/devel/cvsweb.cgi/servers/slapd/proto-slap.h > > Changes are generally available on cvs.openldap.org (and CVSweb) > within 30 minutes of being committed. > -- -- Howard Chu Chief Architect, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Re: commit: ldap/servers/slapd config.h
by Hallvard B Furuseth 12 Mar '08

12 Mar '08

> config.h 1.49 -> 1.50 > const strings This change needs "const char*" or "LDAP_CONST char*", I'm not sure which, in 1st arg to register_at() and register_oc(). -- Hallvard

1 0

Completed SASL canonicalization option patch
by Joel Johnson 11 Mar '08

11 Mar '08

Here is the completed version of the patch (against 2.4.8) I initially brought up a week or so ago. It makes the hostname canonicalization step for SASL optional, both via ldap.conf and commandline arguments for the client tools. I submit it for consideration into the mainline code, as I believe it would provide a great benefit to a number of users such as myself. I present the completed version after some of the comments on the initial snapshot. The majority of comments were, frankly, what I had expected - "fix the DNS, that's the real problem." I argue that such an approach is somewhat narrow. While undoubtedly a best practice, it is not always feasible. I'll share my thoughts and comments on some of the previous responses. I hope to get some good feedback and reconsideration of the knee-jerk reactions. SASL authentication is not strictly dependent on hostname canonicalization in any form. There are, however, certain common implementation schemes that do require a canonical hostname determination in order to function properly. This unfortunately seems to have led to an implicit edict that all DNS configurations must be perfectly compliant in order for OpenLDAP to be able to use SASL authentication. The emphasis is always to 'fix your DNS, then we'll talk', rather than relaxing the overly strict requirement of the canonicalization step - overly strict because it is *not* required from SASL itself, only from OpenLDAP's usage of SASL. There exist broken DNS setups for which a user/admin may not be in a position to remedy the errors. Sorry Howard, saying "just use static /etc/hosts files" or "run your own local authoritative DNS server" are not very viable options. On my home server I have a dynamic IP. When I get a new IP, the machine updates its own /etc/hosts file, and while it would be possible to push the value to others, there are synchronization issues, not the least of which is that the other machines are more than likely not powered on when the update occurs. Running my own DNS server works great when within that netwok, but what about the cases where I'm at work/school/friend/in-laws/wherever and would like to connect? Or when I give access to external users? They certainly aren't going to be using my DNS server as a first line source. Anyway, why should I need any of that additional overhead when alternative, perfectly viable solutions exist. What benefit does the hostname canonicalization procedure provide? In a nutshell, it is simply a method of determining the identity of the entity with whom I'm communicating. When done using DNS it certainly has known security issues, but aside from direct attacks is generally quite reliable if properly configured. I believe that this is why it's not atypical to say "fix your DNS" if something isn't working properly. But why must this be the only option? There are at least two other cases to consider: 1) there are cases where we may not really care one iota about server authentication, and 2) there are options other than DNS for providing, at the very least, the same level of confidence in the identity of the peer. As for the first case, it's not one I'm involved in nor all too familiar with, but it certainly doesn't stretch the imagination to think of a case where the data returned is self-validating or requires no validation. With regard to the second case, alternative host identification, I'm currently using TLS for this, requiring a minimal level of validation. Yes, you can shoot yourself in the foot on this too (no self-signed certs, minimally a cert signed by a self-signed "CA"), but it certainly provides an option where there otherwise might not be one (as in my case). RFC 4752 (SASL GSSAPI), Section 5 states: "When constructing the input_name_string, the client SHOULD NOT canonicalize the server's fully qualified domain name using an insecure or untrusted directory service." I mentioned a similar quote from a less SASL-specific (more Kerberos-specific) RFC before. The emphasis of this statement is to not use improper directory services to perform canonicalization. However, this does *not* imply that canonicalization must be performed, rather it states that *if* the client decides to use it, then a trusted naming service should be used to perform it. RFC 4422 (SASL) makes this explicit and confirms that the canonicalization is at most optional for a client, directly stating that "there is no requirement that the authorization identity string be canonical" (Section 5). Having said all of that, it is also noteworthy to note that canonicalization will often be enforced server-side simply based on the authentication configuration. In the specific case of using Kerberos, if the service principal uses the canonical hostname, the client better be getting a ticket using the canonical hostname as well. As a wrap-up, DNS has issues - some technical and some non-technical. I'm not in anyway suggesting that a proper setup should not be pursued, if such an option is available. But the fact of the matter is that it is not always possible. In such situations, the OpenLDAP software should not overtly preclude the user from using it (or indirectly require large workarounds) to the greatest extent in any situation, even if the configuration of a certain system is not idyllic. A few (or more) thoughts for consideration - Thanks, Joel Johnson

2 1

Re: commit: ldap/libraries/libldap_r tpool.c
by Hallvard B Furuseth 09 Mar '08

09 Mar '08

hyc(a)OpenLDAP.org writes: > tpool.c 1.95 -> 1.96 > ITS#5407 more checks for pool pausing Needs clarification in this comment, at least: "/* See if a pause was requested; wait for it if so." What if more than one thread is waiting to pause? pool_pausecheck() lets through at least one, but not necessarily all. Sufficient for forward progress? Also a thread may request a pause just as pool_pausecheck() returns. With so much common code I'd make this and pool_pause() wrappers around handle_pause(tpool, always_pause), returning !always_pause if it paused. -- Hallvard

2 1

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

openldap-devel