William Tu <u9012...@gmail.com> writes:
> On Tue, Jun 30, 2020 at 12:11 AM Toshiaki Makita
> <toshiaki.maki...@gmail.com> wrote:
>>
>> On 2020/06/30 1:17, 0-day Robot wrote:
>> > Bleep bloop. Greetings Toshiaki Makita, I am a robot and I have tried out
>> > your patch.
>> > Thanks for your contribution.
>> >
>> > I encountered some error that I wasn't expecting. See the details below.
>> >
>> >
>> > checkpatch:
>> > WARNING: Comment with 'xxx' marker
>> > #252 FILE: lib/netdev-afxdp.c:329:
>> > /* XXX: close output_map_fd somewhere? */
>> >
>> > ERROR: Improper whitespace around control block
>> > #734 FILE: lib/netdev-offload-xdp.c:258:
>> > FLOWMAP_FOR_EACH_INDEX(idx, mf->map) {
>>
>> Adding a whitespace like
>>
>> FLOWMAP_FOR_EACH_INDEX (idx, mf->map) {
>>
>> fixes the error, but as far as I can see, all existing usage of this macro
>> does not have this kind of whitespace.
>>
>> Which is correct, need whitespace or not?
Need whitespace. The usage of FLOWMAP_FOR_EACH throughout the codebase
predates automated checking. You'll note that when someone contributes
new versions (ex: tests/oss-fuzz/miniflow_target.c) they have the
whitespace.
Prior to a checkpatch utility, it was more difficult to catch instances
of style violations, and some snuck through. Even with checkpatch, some
make it through (though hopefully fewer).
> I think we need whitespace here.
Yes.
> Similar macros such as FLOWMAP_FOR_EACH_UNIT, FLOWMAP_FOR_EACH_MAP
> also add whitespace
> William
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
