netdev - Re: [PATCH net-next v1 4/6] tools/net/ynl: Add binary and pad support to structs for tc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <m28r6a6iwp.fsf@gmail.com>
Date: Mon, 04 Dec 2023 16:18:14 +0000
From: Donald Hunter <donald.hunter@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org,  "David S. Miller" <davem@...emloft.net>,  Eric
 Dumazet <edumazet@...gle.com>,  Paolo Abeni <pabeni@...hat.com>,  Jonathan
 Corbet <corbet@....net>,  linux-doc@...r.kernel.org,  Jacob Keller
 <jacob.e.keller@...el.com>,  donald.hunter@...hat.com
Subject: Re: [PATCH net-next v1 4/6] tools/net/ynl: Add binary and pad
 support to structs for tc

Jakub Kicinski <kuba@...nel.org> writes:

> On Thu, 30 Nov 2023 21:49:56 +0000 Donald Hunter wrote:
>> The tc netlink-raw family needs binary and pad types for several
>> qopt C structs. Add support for them to ynl.
>
> Nice reuse of the concept of "pad", I don't see why not:
>
> Reviewed-by: Jakub Kicinski <kuba@...nel.org>
>
>> +                value = msg.raw[offset:offset+m.len]
>
> What does Python style guide say about spaces around '+' here?
> I tend to use C style, no idea if it's right.

The relevant section seems to be this:

  However, in a slice the colon acts like a binary operator, and should
  have equal amounts on either side (treating it as the operator with
  the lowest priority). In an extended slice, both colons must have the
  same amount of spacing applied. Exception: when a slice parameter is
  omitted, the space is omitted:

  # Correct:
  ham[1:9], ham[1:9:3], ham[:9:3], ham[1::3], ham[1:9:]
  ham[lower:upper], ham[lower:upper:], ham[lower::step]
  ham[lower+offset : upper+offset]
  ham[: upper_fn(x) : step_fn(x)], ham[:: step_fn(x)]
  ham[lower + offset : upper + offset]

  # Wrong:
  ham[lower + offset:upper + offset]
  ham[1: 9], ham[1 :9], ham[1:9 :3]
  ham[lower : : step]
  ham[ : upper]

On that basis I could change it to:

  (a) value = msg.raw[offset : offset+m.len]

or:

  (b) value = msg.raw[offset : offset + m.len]

But the existing convention in the code is a mix of these styles:

  raw[offset:offset + 4]
  raw[offset:offset+m['len']]

Happy to go with whatever preference, though maximising whitespace per
(b) follows python style _and_ C style?

Also happy to make it consistent across the file (in a separate patch)?