On 11 December 2014 at 15:09, Ben Pfaff <[email protected]> wrote:
> On Thu, Dec 11, 2014 at 02:48:46PM -0800, Joe Stringer wrote:
>> On 11 December 2014 at 14:05, Ben Pfaff <[email protected]> wrote:
>> > On Wed, Dec 03, 2014 at 02:24:46PM -0800, Joe Stringer wrote:
>> >> The UFID parameter to dpif_flow_get() is optional, but the current
>> >> implementation dereferences it to initialize part of the output flow.
>> >> This field is filled in by the dpif implementation, so don't initialize
>> >> it here.
>> >>
>> >> This could cause a NULL dereference if a dpif_flow_get() caller doesn't
>> >> provide a UFID. Currently there are no such callers, but the next patch
>> >> will introduce one.
>> >>
>> >> Signed-off-by: Joe Stringer <[email protected]>
>> >
>> > Acked-by: Ben Pfaff <[email protected]>
>> >
>> > The title of the patch makes this sound like a bug fix, but the
>> > explanation later on sounds like it would only be a real bug if the
>> > callers were changed. Is there a bug that can currently be triggered?
>>
>> No, it cannot be triggered currently. Perhaps this could be rolled
>> into the next patch, or renamed.
>
> I'd just adjust the commit message to make it even clearer. Maybe
> something like "This does not fix any existing bug because every caller
> currently passes in a UFID."
OK, thanks for the review. I will push this series soon with the log
updated as below:
dpif: Don't initialize output UFID in dpif_flow_get().
The UFID parameter to dpif_flow_get() is optional, but the current
implementation dereferences it to initialize part of the output flow.
This field is filled in by the dpif implementation, so don't initialize
it here.
This does not fix any existing bug because every caller currently passes
in a UFID. The next patch will introduce the first call to
dpif_flow_get() that doesn't provide a UFID, which would break without
this change.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
