On 5/5/07, Artem Bityutskiy <[email protected]> wrote:
On Sat, 2007-05-05 at 19:18 +0530, Satyam Sharma wrote:
> Well, you're developing / maintaining this right now, so it's your
> call. Though I bet most people would find keeping that list_add_tail
> local to scan.c more tasteful.
I do not think so. If you are interested, try to find "UBI take 2"
patches in lkml. Look how it looked liked. It consisted of many
independent units and units could access other units _only_ via
interfaces. I would do what you say there.
But what you're doing now is *worse*, don't you see it? You _think_
that you are not exporting any add_to_list function from scan.c
because you removed the wrapper from scan.h, but then you open-code it
anyway in create_vtbl (yes, that list_add _is_ the *only* meaningful
code in add_to_list; it just so _happens_ that most of its callers
till now also wanted to allocate a ubi_scan_leb too at that time so
you pushed that too into add_to_list rather than do it in the callers
-- via a separate alloc_leb, for example). Now that means there is an
*undocumented* use of the "add to list" _functionality_ outside of
scan.c anyway and which wouldn't even come up if someone tries to
analyse / overhaul the code some day in the future. I know it's just
_one_ exception, but still, I'm a heckler for style.
Read Teo's comments - I actually now agree with them. And after I had
changed UBI i got rid of several thousands lines of code, and the code
became simpler.
So, my argument is:
1. It makes no sense to introduce one more non-static function to _just_
encapsulate list_add_tail and _just_ for one caller.
Well, introducing one more non-static function is *not* what I
originally had in mind (it was proposed only as a temporary measure
till all users of ubi_scan_add_to_list could be updated to use the
other version, explained below).
I actually planned to _replace_ ubi_scan_add_to_list with another
version that does *not* allocate memory (the __ubi_scan_add_to_list I
proposed above), and leave allocation up to its *callers* who
therefore pass a valid ubi_scan_leb to it. But that is where I ran
into ubi_scan_add_used().
2. It is _C_, it is _kernel_, and it is OK sometimes _not_ to follow
computer since rules.
Ah, well, forget it. It's all a matter of taste and maintainability. I
guess in the end it needs to be fine with you.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
