Wojcik, Artur wrote:
And I don't like the introduction of a new header file just to store one or 2
definitions. Just put new definitions in mdadm.h Keep It Simple.
Sorry but I don't agree. At the moment the source code of mdadm/mdmon is
not simple.
Adding a new header file for a few definitions does not reduce complexity.
It's quite complicated. It is hard to analyze how the
control flows and there's a lot of data and code redundancy in it. In my
opinion "refactorization" is what this code needs. Splitting code into
files would be helpful, too.
The current file split is pretty simple and self explanatory. Each
major operating mode of mdadm has its own file.
I will say that there are some long functions like Assemble() that could
use some helper routines to split the major stages. If only to add
documentation points and clarify the current working set of parameters.
That being said I have only had to touch Assemble.c a handful of
times, so the cost-benefit ratio of doing this refactoring never became < 1.
If you find some redundant code that would be a good starting point for
simplification patches.
I am perfectly happy with making 'buf' larger as appropriate, even making it
PATH_MAX in some cases.
I would be happy with more use of asprinf.
I would be happy adding checks before certain critical sprintf calls that
the result will not exceed the buffer.
I did not use asprintf function, because it's GNU specific. I had an
impression that one may try to build mdmdm using non-GNU toolchain.
It is safe to assume that mdadm will always run on a system with glibc,
uclibc, or an equivalent library that has an asprintf() implementation.
--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
