Re: [Xen-devel] [PATCH v2 05/20] rb_tree: remove redundant if()-condition in rb_erase()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.06.17 at 18:53, <dario.faggioli@xxxxxxxxxx> wrote:
> On Sat, 2017-06-17 at 15:02 +0530, Praveen Kumar wrote:
>> --- a/xen/common/rbtree.c
>> +++ b/xen/common/rbtree.c
>> @@ -250,15 +250,16 @@ void rb_erase(struct rb_node *node, struct
>> rb_root *root)
>>              if (child)
>>                  rb_set_parent(child, parent);
>>              parent->rb_left = child;
>> +
>> +            node->rb_right = old->rb_right;
>> +            rb_set_parent(old->rb_right, node);
>>          }
>>  
>>          node->rb_parent_color = old->rb_parent_color;
>> -        node->rb_right = old->rb_right;
>>          node->rb_left = old->rb_left;
>>  
> ...In the Linux commit, this blank line is removed too.
> 
>>          rb_set_parent(old->rb_left, node);
>> -        if (old->rb_right)
>> -            rb_set_parent(old->rb_right, node);
>> +
>>          goto color;
>>      }
>>  
> I don't think this is too big of a deal per se, I'd I'd leave to
> maintainers and committers to decide whether something like this is
> enough for asking a resend, or whether it can be fixed upon commit or
> even left as it is.
> 
> For sure, we know that these patches really needs to be, as much as
> possible, 1:1 copies of Linux's ones, even in the smallest detail. And
> the reason is to make the life of someone wanting to do another round
> of import, in future, as easy as possible.

Yes, and it is for this very reason that I'd like to see a resend. Else
there's the risk that fixing this up upon commit causes subsequent
patches to also not apply. The committer, if doing any edits at all,
should not be required to spend meaningful amounts of extra time
on applying a patch (or series).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel