Hi everyone,
I've been acquainting myself with SSSD code the last few weeks. Particularly,
the NSS responder, and what it uses down the stack.
The SSSD code is sure complex and its asynchronous nature doesn't help.
However, what struck me, and frustrated me the most, as a new reader of the
code, was the meager amount of code documentation.
In the absence of documentation, a piece of logic, such as processing a
getpwnam request, requires traversing many functions, modules, and several
programs. Sure, you can guess what a function does, based on its name and
somewhat on arguments, but you can't be sure without reading it and the
functions it calls, and often the functions those call in turn. Same goes with
data structures: to understand them you need to read the code that creates and
operates them.
To the author code often seems obvious, but a new reader has a dozen
questions: which state the passed object should be in, how does the function
modify the object, where does it put its output, what else it affects, exactly
what string should be that, what status codes the function can return and what
will they mean, etc.
Documentation cements the author's intention in the mind of the reader,
freeing the memory and precious attention for understanding the code at hand,
as opposed to requiring going deeper and deeper into the underlying code,
where the reader eventually forgets why he or she is looking at it. Reading
such complicated code as can be seen in SSSD without documentation is an
exercise in frustration.
We often speak about cooperation and getting more external contributors.
However, we're not going to get many if they have such a mountain of code to
climb without our help. We ourselves will be (and are) struggling to
understand our own code, and will pay with lost time again and again when
trying to modify or extend it.
In short: it is NOT OK that SSSD code has no documentation.
Put bluntly, it is sloppy engineering. I'm sure we can do better.
If you have just a sentence describing each function and each of its
arguments, understanding the code becomes considerably easier. You don't have
to guess that much, and you don't have to verify your guesses and read so much
more. Function documentation would often make reading its code unnecessary.
A sentence for a data structure and each of its fields makes understanding the
code operating it much easier.
I suggest several measures which could be taken.
1. Require *every* new module/function/argument/structure/field to be
documented. It must be at least a sentence for each. A module or a bigger
data structure could warrant more. Describe module/function/structure
overall, and each of its declarations/arguments/return values/fields.
2. Next level, documentation tax: if you touch anything, document it. Modified
a module header? Add a short module description. Added or removed a field
in a structure? Add structure and field descriptions. Changed a function?
Document it.
3. Third level, documentation duty: document a whole existing module header of
your choosing, say once a month.
The format of documentation doesn't matter, as long as it's legible and agreed
upon. I suggest Doxygen, but it can be anything else. We don't have to
actually run the documentation extraction tool (such as "doxygen") on our code
and use perfectly correct syntax (although that could be the next step). What
matters is that documentation is there, it's uniform, and you don't have to
think about the format when writing it.
For example of how it can look see the attached patch where I tried to
document some things (likely misunderstanding some) as I read the code.
For more examples of what I mean see the tlog header files:
https://github.com/Scribery/tlog/tree/master/include/tlog
They are not the best example and there are many ways they can be improved
(tell me please!), but if SSSD had just that it would be already awesome.
Please tell me what you think, object, or suggest your own solutions!
Thank you.
Nick