Since sdap_async_accounts.c is becoming a great monstrosity, I've created a call graph of functions defined in there. The graph shows where in the file is each function defined there called and it already takes Jakub's last big set of patches into account. I'm also sending the source .dot file in case anyone wants to extend it in the future.
I hope it will be useful to more people than just me.
Thanks Jan
On Fri, 2011-05-06 at 13:10 +0200, Jan Zelený wrote:
Since sdap_async_accounts.c is becoming a great monstrosity, I've created a call graph of functions defined in there. The graph shows where in the file is each function defined there called and it already takes Jakub's last big set of patches into account. I'm also sending the source .dot file in case anyone wants to extend it in the future.
I hope it will be useful to more people than just me.
I think this proves beyond a shadow of a doubt that we need to split this file up into the following (much) smaller files:
1) sdap_async_accounts_users.c - Common functions for processing user lookups 2) sdap_async_accounts_initgroups.c - Common functions for processing initgroups 3) sdap_async_accounts_initgroups_2307.c - The real workhorse for initgroups on RFC2307 servers 4) sdap_async_accounts_initgroups_2307bis.c - The real workhorse for initgroups on RFC2307bis servers 5) sdap_async_accounts_groups.c - Common functions for processing groups 6) sdap_async_accounts_groups_2307.c - RFC2307-specific group processing 7) sdap_async_accounts_groups_2307bis.c - RFC2307bis-specific group processing
I have opened https://fedorahosted.org/sssd/ticket/864 to track this.
On Fri, 2011-05-06 at 13:10 +0200, Jan Zelený wrote:
Since sdap_async_accounts.c is becoming a great monstrosity, I've created a call graph of functions defined in there. The graph shows where in the file is each function defined there called and it already takes Jakub's last big set of patches into account. I'm also sending the source .dot file in case anyone wants to extend it in the future.
I hope it will be useful to more people than just me.
I think this proves beyond a shadow of a doubt that we need to split this file up into the following (much) smaller files:
- sdap_async_accounts_users.c - Common functions for processing user
lookups 2) sdap_async_accounts_initgroups.c - Common functions for processing initgroups 3) sdap_async_accounts_initgroups_2307.c - The real workhorse for initgroups on RFC2307 servers 4) sdap_async_accounts_initgroups_2307bis.c - The real workhorse for initgroups on RFC2307bis servers 5) sdap_async_accounts_groups.c - Common functions for processing groups 6) sdap_async_accounts_groups_2307.c - RFC2307-specific group processing 7) sdap_async_accounts_groups_2307bis.c - RFC2307bis-specific group processing
I have opened https://fedorahosted.org/sssd/ticket/864 to track this.
I'd also add some *_common file(s), because some routines are used in more parts of the code
Jan
On Fri, 2011-05-06 at 07:55 -0400, Stephen Gallagher wrote:
On Fri, 2011-05-06 at 13:10 +0200, Jan Zelený wrote:
Since sdap_async_accounts.c is becoming a great monstrosity, I've created a call graph of functions defined in there. The graph shows where in the file is each function defined there called and it already takes Jakub's last big set of patches into account. I'm also sending the source .dot file in case anyone wants to extend it in the future.
I hope it will be useful to more people than just me.
I think this proves beyond a shadow of a doubt that we need to split this file up into the following (much) smaller files:
- sdap_async_accounts_users.c - Common functions for processing user
lookups 2) sdap_async_accounts_initgroups.c - Common functions for processing initgroups 3) sdap_async_accounts_initgroups_2307.c - The real workhorse for initgroups on RFC2307 servers 4) sdap_async_accounts_initgroups_2307bis.c - The real workhorse for initgroups on RFC2307bis servers 5) sdap_async_accounts_groups.c - Common functions for processing groups 6) sdap_async_accounts_groups_2307.c - RFC2307-specific group processing 7) sdap_async_accounts_groups_2307bis.c - RFC2307bis-specific group processing
This looks excessive to me, probably ok to do just: sdap_async_users.c sdap_async_initgroups.c sdap_async_groups.c
(yes I am dropping 'accounts' at this point as it is redundant)
Simo.
On Fri, 2011-05-06 at 09:03 -0400, Simo Sorce wrote:
On Fri, 2011-05-06 at 07:55 -0400, Stephen Gallagher wrote:
On Fri, 2011-05-06 at 13:10 +0200, Jan Zelený wrote:
Since sdap_async_accounts.c is becoming a great monstrosity, I've created a call graph of functions defined in there. The graph shows where in the file is each function defined there called and it already takes Jakub's last big set of patches into account. I'm also sending the source .dot file in case anyone wants to extend it in the future.
I hope it will be useful to more people than just me.
I think this proves beyond a shadow of a doubt that we need to split this file up into the following (much) smaller files:
- sdap_async_accounts_users.c - Common functions for processing user
lookups 2) sdap_async_accounts_initgroups.c - Common functions for processing initgroups 3) sdap_async_accounts_initgroups_2307.c - The real workhorse for initgroups on RFC2307 servers 4) sdap_async_accounts_initgroups_2307bis.c - The real workhorse for initgroups on RFC2307bis servers 5) sdap_async_accounts_groups.c - Common functions for processing groups 6) sdap_async_accounts_groups_2307.c - RFC2307-specific group processing 7) sdap_async_accounts_groups_2307bis.c - RFC2307bis-specific group processing
This looks excessive to me, probably ok to do just: sdap_async_users.c sdap_async_initgroups.c sdap_async_groups.c
(yes I am dropping 'accounts' at this point as it is redundant)
I would much prefer that the RFC2307 and RFC2307bis pieces be separated out.
On Fri, 2011-05-06 at 15:22 +0200, Jakub Hrozek wrote:
On 05/06/2011 03:12 PM, Stephen Gallagher wrote:
I would much prefer that the RFC2307 and RFC2307bis pieces be separated out.
+1 they are really separate code paths.
Then we should rather split along those lines instead of users vs initgroups vs groups.
Splitting along both lines is too much imo.
We are not changing the complexity of the operations after all only trying to make each file shorter and more manageable. So the path of least resistance should be the path that forces people to jump between files less when looking at stuff.
Simo.
On Fri, 2011-05-06 at 09:03 -0400, Simo Sorce wrote:
On Fri, 2011-05-06 at 07:55 -0400, Stephen Gallagher wrote:
On Fri, 2011-05-06 at 13:10 +0200, Jan Zelený wrote:
Since sdap_async_accounts.c is becoming a great monstrosity, I've created a call graph of functions defined in there. The graph shows where in the file is each function defined there called and it already takes Jakub's last big set of patches into account. I'm also sending the source .dot file in case anyone wants to extend it in the future.
I hope it will be useful to more people than just me.
I think this proves beyond a shadow of a doubt that we need to split this file up into the following (much) smaller files:
- sdap_async_accounts_users.c - Common functions for processing user
lookups 2) sdap_async_accounts_initgroups.c - Common functions for processing initgroups 3) sdap_async_accounts_initgroups_2307.c - The real workhorse for initgroups on RFC2307 servers 4) sdap_async_accounts_initgroups_2307bis.c - The real workhorse for initgroups on RFC2307bis servers 5) sdap_async_accounts_groups.c - Common functions for processing groups 6) sdap_async_accounts_groups_2307.c - RFC2307-specific group processing 7) sdap_async_accounts_groups_2307bis.c - RFC2307bis-specific group processing
This looks excessive to me, probably ok to do just: sdap_async_users.c sdap_async_initgroups.c sdap_async_groups.c
(yes I am dropping 'accounts' at this point as it is redundant)
I would much prefer that the RFC2307 and RFC2307bis pieces be separated out.
I think this will be influenced by possible function merging (which I mentioned in the ticket). I'll analyze the situation and let you know which approach seems to be better.
Jan
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Jan Zelený -
Simo Sorce -
Stephen Gallagher