Daniel Walker wrote:
>
> I think firstly, if you want to have success with this patch you'll need
> to clean it up a bit. I'm not an authority on clean code , but below
> isn't clean to my eyes.
Cleanups are always good, I am open to any suggestions.
> However, this is cleaner than your last
> attempt .
Thanks, but it was NOT changed from my last attempt. Just rediff.
> Hard coding MAX_PRIO isn't really acceptable.
I don't do that? Could you clarify?
> If your going to make it
> more similar to list_head , why not name it plist_head instead of
> pl_head that way it's easy to switch between them.
I don't mind to rename, probably plist_head is better. I'd like to know
Ingo's opinion first.
> Like you
> remove a lot of the API which makes it less similar to a regular list .
For example?
> Also, making any changes to the internals of the plist structure outside
> of plist.c (or similar) isn't acceptable. For instance you set the node
> priority in several places, that should be hidden inside another
> function or macro. That makes it easier for people to change the
> internal structure without treading though tons of code.
Agreed, I already thought it makes sense to add plist_add_prio() helper.
Note that ->prio is set directly mostly right before plist_add() call.
> Changing plist_empty() doesn't make any sense to me.
plist_empty(head) means this list empty. plist_unhashed(node) means
this node is not on list.
> Also changing
> dp_node to prio_list doesn't make much sense either.
Again, this patch is unchanged. I don't mind to rename, but recall
that it was sent before the current implementation become functional.
And honestly I don't like 'sp_node', this name is misleading, and
reflects first buggy implementation. It should be called 'all_nodes'
or something like this.
Oleg.
-
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]
