Thanks for the review!
Pushed with the change that classifier_remove takes a const struct cls_rule
pointer as an argument.
Jarno
On Nov 6, 2014, at 11:58 AM, Ben Pfaff <b...@nicira.com> wrote:
> On Thu, Nov 06, 2014 at 11:29:26AM -0800, Jarno Rajahalme wrote:
>>
>> On Nov 6, 2014, at 11:08 AM, Ben Pfaff <b...@nicira.com> wrote:
>>
>>> On Thu, Nov 06, 2014 at 11:06:59AM -0800, Ben Pfaff wrote:
>>>> On Thu, Nov 06, 2014 at 11:02:56AM -0800, Ben Pfaff wrote:
>>>>> On Mon, Nov 03, 2014 at 11:39:00AM -0800, Jarno Rajahalme wrote:
>>>>>> Returning const struct cls_rule pointers from the classifier API helps
>>>>>> callers to remember that they should not modify the rules returned.
>>>>>>
>>>>>> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
>>>>>
>>>>> I don't think it has much practical effect since most of the callers
>>>>> immediately cast the pointer to some other container type, but it does
>>>>> seem a little nicer.
>>>>>
>>>>> Acked-by: Ben Pfaff <b...@nicira.com>
>>>>
>>>> But you'll need to fix up ovs-router.c, which I think is new since you
>>>> posted this. It has some build failures (GCC and sparse output
>>>> interleaved below):
>>>>
>>>> ../lib/ovs-router.c:142:8: warning: incorrect type in assignment
>>>> (different modifiers)
>>>> ../lib/ovs-router.c:142:8: expected struct cls_rule *cr
>>>> ../lib/ovs-router.c:142:8: got struct cls_rule const *
>>>> ../lib/ovs-router.c:145:12: warning: incorrect type in assignment
>>>> (different modifiers)
>>>> ../lib/ovs-router.c:145:12: expected struct cls_rule *cr
>>>> ../lib/ovs-router.c:145:12: got struct cls_rule const *
>>>> ../lib/ovs-router.c: In function 'rt_entry_delete':
>>>> ../lib/ovs-router.c:142:8: error: assignment discards 'const' qualifier
>>>> from pointer target type [-Werror]
>>>> cr = classifier_find_rule_exactly(&cls, &rule);
>>>> ^
>>>> ../lib/ovs-router.c:145:12: error: assignment discards 'const' qualifier
>>>> from pointer target type [-Werror]
>>>> cr = classifier_remove(&cls, cr);
>>>> ^
>>>
>>> And actually the code in question makes one wonder whether we should
>>> either abandon this change or make classifier_remove() take a const
>>> pointer too:
>>>
>>> cr = classifier_find_rule_exactly(&cls, &rule);
>>> if (cr) {
>>> /* Remove it. */
>>> cr = classifier_remove(&cls, cr);
>>> if (cr) {
>>
>>
>> I think I bumped into this myself, making classifier_remove to take a
>> const pointer works, so that’s what I’d like to do.
>
> Fine with me, thanks.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
