On Mon, 31 May 2010 23:47:40 +0100, "Markus Moeller" <[email protected]> wrote: > "Amos Jeffries" <[email protected]> wrote in message > news:[email protected]... >> On Sun, 30 May 2010 16:15:14 +0100, "Markus Moeller" >> <[email protected]> wrote: >>> Hi, >>> >>> I have converted my helper to kerberos_ldap_group ( not sure if that >> is >>> the best name) and created a patch for inclusion into the head revision. >> >>> Please review and let me know any feedback. >>> >>> >>> Thank you >>> Markus >> >> Hi Markus, >> >> * No need to include the COPYING file when bundled, Squid gets >> distributed with a version in the base directory. > > OK I removed it > >> * Also seems to have pulled in "negotiate_kerberos_auth-logging.patch" >> and "negotiate_kerberos_auth-new.cc" >> > > YES as I think their are not required just a left over > >> >> support.h: >> * config.h will pull in the compat/* files. no need to include them >> explicitly now. it will also pull in several of the standard headers. > > OK I removed them > >> * not sure about that LogTime definition going in here when its not >> inline. Can it be moved to a .cc? >> > > YES I did create another support_log.cc > >> kerberos_ldap_group.cc and support_*.cc: >> * needs to include config.h as the first include, this will pull in the >> basic headers for types etc for portability: unistd.h, stdlib.h, >> sys/time.h, etc >> > > OK. Done > >> * ALL system headers used need to be wrapped with HAVE_* even if standard >> headers, and go below the local file includes. If a .h file needs some >> definition to build, the system file needs to be pulled into there >> instead >> of the .cc. >> > > OK Done > >> Also; why are the support_*.cc files all in helpers/external_acl/ >> directly? >> > > Apologies a wrong copy in my source. > >> support_ldap.cc: >> * sub-Copyright for The OpenLDAP Foundation will have to be added to the >> Squid combined CREDITS copyright file. >> > > I updated CREDITS. > > >> support_netbios.cc: >> * I thought NetBOIS was officially declared dead by MS? can that be >> dropped? >> > > This is for cases when I get a user name of NETBIOS-DOMAIN\user (e.g. NTLM > auth) so I can transform it into u...@kerberos-domain. Unfortunately the
> NETBIOS-DOMAIN and KERBEROS-DOMAIN names are not always the same. Oh well. Okay. here to hoping :) > >> support_resolv.cc: >> * RFC1035 text is bundled with Squid in docs/rfc/, IMO a simple reference >> to the RFC number and section numbers should be sufficient for the >> documentation. > > OK. I did not see the rfc directory in my rsync copy. This was more for my > own guidance > >> * RFC 2782 can be pulled into doc/rfc/ and added to that >> doc/rfc/1-index.txt as a separate patch instead of quoting large >> portions. > > OK. as above > >> * There are rfc1035 type definitions and packers/unpackers are provided >> through $(COMPAT_LIB) for now. see include/rfc1035.h if you actually need >> them. Though from a quick look you seem to be using the resolver library >> API not packet-level details from RFC 1035. > > I did not know that they were available as I created the module > independant > of squid libraries. I will check how to use them. > >> * NP: long-term plan is to have a libdns in Squid you can link and call, >> but that is not ready yet, so overlooking the manual resolving this file >> does for now. >> > > So you will include resolving SRV records ? There is a TODO list entry to add them, yes. This is long-term and will happen after this helper gets included. > >> * Some good news; since we imported the auth helper I've cleaned up >> Squids' API for getaddrinfo/freeaddrinfo/gai_strerror. You can use the >> POSIX names now instead of the xgetaddrinfo variants. >> > > Excellent > >> >> Amos >> > > I think we have to change the config.test. It really does not cover cases > where the header files are not in the default place (.e.g. if krb5-config > points to a newer library) > > Thank you very much for the quick and detailed reply > Welcome. Thanks for the helper. Amos
