sssd-devel August 2009

sssd-devel@lists.fedorahosted.org

11 participants
80 discussions

[PATCH] Trace updates (Ticket 99)
by Dmitri Pal 17 Aug '09

17 Aug '09

Addressing ticket: https://fedorahosted.org/sssd/ticket/99 See comments in patch. -- Thank you, Dmitri Pal Engineering Manager IPA project, Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/

3 2

[PATCH] Fix DP reconnection code [#101]
by Simo Sorce 17 Aug '09

17 Aug '09

Fix reconnection mess caused by double reconnection code. Remove one of the reconnection facilities and make code more uniform. Fixes ticket 101 Simo.

2 3

[SSSD WIKI Update] Information about tools used in SSSD
by Dmitri Pal 14 Aug '09

14 Aug '09

Hello, SSSD tree contains a set of common interfaces that provide convenient API for different operations. In future we plan to turn some of those tools into separate deliverables. For now they are a part of the SSSD. See details on the SSSD wiki page: https://fedorahosted.org/sssd/wiki/WikiPage/SSSDTools I am in charge of "Collection", "INI" and "ELAPI" parts so far so I will keep those updated. Any comments and suggestions are welcome. -- Thank you, Dmitri Pal Engineering Manager IPA project, Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/

1 0

[Fwd: [seceng-idm-qe-list] SSSD Testing Status]
by Jenny Galipeau 14 Aug '09

14 Aug '09

-- Jenny Galipeau <jgalipea(a)redhat.com> Principal Software QA Engineer Red Hat, Inc. Security Engineering

4 6

[PATCH] Make "files" a reserved word for legacy local domain
by Jakub Hrozek 14 Aug '09

14 Aug '09

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 This patch introduces provider=files as a valid provider. Upon loading the backend, its properties in confdb are overwritten to those that represent legacy local domain. Also get rid of old hackish way of detecting legacy local domain in tools Jakub -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkqC+4gACgkQHsardTLnvCV/FACg1PDQiDXyLTO4KFzuuitCjV0T DFEAn3O56agl+Q570u3/vLfGomIGFEHA =JDTz -----END PGP SIGNATURE-----

4 7

Bringing 0.5.0 to a close
by Stephen Gallagher 14 Aug '09

14 Aug '09

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 To all developers and integrators. We have scheduled our intent to release SSSD 0.5.0 on Monday, August 24th. This is ten calendar days from today. If you are working on a feature or bugfix that you believe to be critical to the release of 0.5.0, please ensure that there is a ticket logged in Trac(1) with the priority of "blocker". On Friday, August 21st, I will review all open bugs. If there are no blockers listed, we will freeze upstream commits over the weekend for testing. On Monday morning, if there are no show-stopping bugs found, we will tag, sign and release 0.5.0. I urge all developers to please update their tickets to denote whether they feel they are needed for a stable release. 1: https://fedorahosted.org/sssd/report/1 - -- Stephen Gallagher RHCE 804006346421761 Looking to carve out IT costs? www.redhat.com/carveoutcosts/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkqFZSgACgkQeiVVYja6o6NFjgCgrkX16jW1mR/KV5lJj9pS3Uot c08Anjw/SD6NLSTasxHosaFOUZqJ4n+e =0+8W -----END PGP SIGNATURE-----

1 0

[PATCH] Don't go to the backend for identical cache entry requests
by Stephen Gallagher 14 Aug '09

14 Aug '09

Currently, if an additional request comes in for a cache entry while that same entry is already in the process of being refreshed, we start a duplicate cache update request. This patch adds allows the cache to maintain a hash table of all in-progress requests and queue up multiple callbacks for updates in progress. Once the data is returned, all of these callbacks will fire. Because requests can carry multiple callbacks, we cannot rely on the life of the current caller to manage the memory for the request. We'll manage the memory ourselves internally and remove the mem_ctx argument from nss_dp_send_acct_req() so there's no ambiguity. This patch also introduces the necessary framework to implement the out-of-band cache updates (which will follow in a subsequent patch) -- Stephen Gallagher RHCE 804006346421761 Looking to carve out IT costs? www.redhat.com/carveoutcosts/

2 3

[PATCH] Tools ID range fixes
by Jakub Hrozek 13 Aug '09

13 Aug '09

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Patch attached. Jakub -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkqC9KgACgkQHsardTLnvCUiLgCg10+VZwOr4hYAzshueEri6517 /LMAnRVXuFlhSW/SDB2PhL5DjNsdanHh =eYfp -----END PGP SIGNATURE-----

3 2

[PATCH] Move parsing parameters in tools
by Jakub Hrozek 13 Aug '09

13 Aug '09

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 See commit message, basically just moves parsing of params up to set debug_level before calling anything that might emit DEBUG() messages (like initialization of the tools). Also fixes one typo where sss_groupmod had USERNAME in help string. Jakub -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkqD/xYACgkQHsardTLnvCXr1QCfSpJ8HmzBooIVACtPBkR29LKx iyYAn05zhURtZI3G/PmlEFDzHl5QTiI3 =2Jfm -----END PGP SIGNATURE-----

1 1

Re: [SSSD] [Freeipa-devel] [PATCHES] one for INI another for ELAPI
by Dmitri Pal 12 Aug '09

12 Aug '09

> > Nack. > > Please squash the patch "ELAPI Adding Core Functionality" into this > second patch. Neither one makes sense without the other. > Done > I don't like the --with-config-file and --with-config-dir configure > parameters. It's usually preferred to keep the filename constant (unless > overridden at runtime by a user) and just specify its location. > > I'd suggest using the following: > --with-config-dir [SYSCONFDIR/elapi] > --with-extra-config-dir [SYSCONFDIR/elapi/apps.d] > Done > You can't pass APP_NAME_SIZE as a string and then compare > strlen(appname) to it. You're comparing it to the pointer address of the > static string. You need to convert it to an integer either in the > configure script or in the C code before you can use it that way. > This is not true. In the make file the defined value is not in quotes: ... -DELAPI_DEFAULT_APP_NAME=\"default\" -DELAPI_DEFAULT_APP_NAME_SIZE=127 ... The strings are in quotes so they are defined as strings but if you do not specify quotes you get number. So the ELAPI_DEFAULT_APP_NAME_SIZE is a number and the comparison works correctly. I also added trace and printed it as number and got expected result: [DEBUG] elapi_log.c ( 146) ELAPI_DEFAULT_APP_NAME_SIZE = 127 This is not an issue. > --with-error-file doesn't make sense. There's no reason they should need > to change the name of the file. Possibly its location (if they want it > somewhere other then CWD) I do not see the reason why it should be somewhere else other than CWD. It seemed reasonable to be to have it defined. But it its a standard to hard code such things so be it. I removed the option from configure and makefile and #defined name in the source. Brrr... IMO from better to worse... > > Your ELAPI async interface is incomplete and belongs in a separate patch > in any case. I will review this in more detail separately. I can't remove it from the patch. I tried. It is used in the core code inside the elapi_create_dispatcher_adv() function. This is the minimal first pass at this function. It will be enhanced more going forward but it is sufficient for now. The elapy_async.h has munimal set of typedefs for callbacks - nothing else. It is still included. > elapi_event.c: > add_host_identity() > Please don't use memset on large static character arrays. It's a > wasteful operation (in this case, it's 1025 memory writes where one is > sufficient). It's much better to set hostname[0] = 0; and then just > ensure null termination later if you copy data into it. Memsetting a > character array leads to the bad assumption that you are guaranteed a > termination. For example, when you use gethostname() later, if the > hostname is longer than NI_MAXHOST for some reason, you are not > guaranteed null termination. So you should manually insert a null > terminator at hostname[NI_MAXHOST]. You do the same thing in several > other places in this function, and throughout your code. getnameinfo() > guarantees null termination, so memset is completely wasted here. Well if you claim we can trust it (which I did not) then I will remove memset. I usually do not trust functions to claim that they return NULL terminated strings until I get the proof for it. getnameinfo() is relatively new function at least for me. I am used to old interface gethostname, gethostbyaddr etc. Those were not trusted. But things change... Ok, done. > > getifaddrs() is not portable. It is implemented only for systems > corresponding to BSDi, and even on those systems, it varies widely. If > you intend to use this, it needs to be controlled by a configure flag. > But this is what I was told to use to get reliable address data. I will open a ticket to make it portable. It grants a separate effort. Opened ticket: https://fedorahosted.org/sssd/ticket/94 This function is supported by Ubuntu http://manpages.ubuntu.com/manpages/karmic/man3/getifaddrs.3.html so we are safe to have it as is for now until the ticket is addressed later this month. > Please prefer comparisons to EOK or EXIT_SUCCESS instead of 0. > Comparison to 0 can imply "false". Okey. EOK then. > Similarly, > if (!gethostname(hostname, NI_MAXHOST)) hnm = hostname; > doesn't read well. > Fixed. > Comparing against LOCALHOSTDOMAIN is unnecessary, because it will > already have matched LOCALHOST (since you are limiting the search to the > length of LOCALHOST). I'd rather see you actually perform a strcasecmp > rather than a strncasecmp here, because it's theoretically possible that > someone could choose localhost1.example.com for some ridiculous reason, > and we'd be throwing it away. It's safe to omit the search limit because > strcasecmp() will stop at the end of the shorter string. > Changed to strcasecmp() though I do not like it. > Why are you doing a string compare of the printable versions of the > address vs. LOCALADDRESS[V6]? It would be much faster to do a numeric > compare of sockaddr_in->in_addr->s_addr; > Yes it would but there are some reasons to do it as strings. For example there are odd cases when "127:0:0:1" is returned as name. To catch these cases it is better to convert names and addressed to strings. In this case you can compare them easily and do not think about IPv4 vs. IPv6. I know that it is a bit of cost but this is a template operation. It is expected that templates are created infrequently, probably once per lifetime of the application. We discussed this with Steven and agreed that this is acceptable as is. > Similarly, why are we storing the host address as a string instead of as > an integer? Seems like a waste of space. > Yes but see above. You need to catch cases when name and address return same thing. This is the indication of the odd case. You do not want to have all events have duplicate hostname and address - it is a much bigger waste of space and cycles. > add_base_elements(): > pass_base = base; is an implicit signed/unsigned conversion on some > platforms. It would be best if you defined them both as uint32_t. > Made it unsigned. Do not see a reason to change it to uint32_t. > interprete_key(): > For the sake of the sanity of anyone using the code, please spell > "interpret" correctly. Done > There's no need to return the length of the > strings in the function, since you know they're null-terminated. > Returning the length is only useful in arrays without null-termination. > This is wrong, the whole point of returning length is that it is NOT null terminated and we can't alter the buffer we are reading from. This is why we create a copy later on. So it is essential to return the length in such cases as you underlined above. > convert_bool(): > I'd prefer that you use strcasecmp rather than strncasecmp unless you > want to allow "Truest" and "Falsely" to match TRUE and FALSE as well. > Ohhh... Ok, done > process_arg_list(): > strndup() is a GNU extension, it is not portable. Since you control > interpret_key(), you know that property will never be unreasonably large > and will always be null-terminated, so it's safe to use strdup() here. It is not null terminated. The whole reason of creating a copy is because we need to NULL terminate it but we cant alter the source string. I changed strndup to malloc + memcpy + NULL terminate. > Use the aforementioned ELAPI_KEY_TYPE_* defines here instead of just > if/else. Do not understand the comment, seems incomplete. > > > Prefer "elapi_create_event_va" to "with_args". Both elapi_create_event() > and elapi_create_event_with_args() take arguments. > > Renamed all to be elapi_XXX_with_vargs() > > elapi_internal.c: > This is a matter of personal preference, but I prefer > _elapi_sink_handler() over elapi_int_sink_handler() to denote an > internal function. I am aware that we have an _int suffix in some places > in the code, but I'd like to recommend that we move to the underscore > prefix. I will send out a coding style change request separately. > I never liked the _ as a prefix so to avoid the confusion I expanded all _int_ suffixes to _internal_ to avoid confusion. > elapi_log.c: > No issues beyond those mentioned above. > Run the the whole thing in valgrind, found couple issues and addressed them. Thanks for review! New patches are attached. > > ------------------------------------------------------------------------ > > > _______________________________________________ > > Freeipa-devel mailing list > > Freeipa-devel(a)redhat.com > > https://www.redhat.com/mailman/listinfo/freeipa-devel > > _______________________________________________ Freeipa-devel mailing list Freeipa-devel(a)redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Thank you, Dmitri Pal Engineering Manager IPA project, Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/

4 12

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

sssd-devel August 2009