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: <87o7m04tq5.fsf@nvidia.com>
Date: Wed, 31 May 2023 13:26:06 +0200
From: Petr Machata <petrm@...dia.com>
To: Daniel Machon <daniel.machon@...rochip.com>
CC: Petr Machata <petrm@...dia.com>, <netdev@...r.kernel.org>,
	<dsahern@...nel.org>, <stephen@...workplumber.org>,
	<UNGLinuxDriver@...rochip.com>
Subject: Re: [PATCH iproute2-next v2 2/8] dcb: app: modify dcb-app print
 functions for dcb-rewr reuse


Daniel Machon <daniel.machon@...rochip.com> writes:

>> Daniel Machon <daniel.machon@...rochip.com> writes:
>> 
>> > Where dcb-app requires protocol to be the printed key, dcb-rewr requires
>> > it to be the priority. Adapt existing dcb-app print functions for this.
>> >
>> > dcb_app_print_filtered() has been modified, to take two callbacks; one
>> > for printing the entire string (pid and prio), and one for the pid type
>> > (dec, hex, dscp, pcp). This saves us for making one dedicated function
>> > for each pid type for both app and rewr.
>> >
>> > dcb_app_print_key_*() functions have been renamed to
>> > dcb_app_print_pid_*() to align with new situation. Also, none of them
>> > will print the colon anymore.
>> >
>> > Signed-off-by: Daniel Machon <daniel.machon@...rochip.com>
>> 
>> There are about four patches included in this one patch: the %d->%u
>> change, the colon shenanigans, the renaming, and prototype change of
>> dcb_app_print_filtered().
>> 
>> I think the code is OK, but I would appreciate splitting into a patch
>> per feature.
>
> Actually, IMHO, the 'colon shenanigans' does not make much sense in a
> patch of its own, since its not really a standalone feature, but rather
> some change that is required for the prototype change of
> dcb_app_print_filtered? 

It doesn't make sense on its own, but it also does not get sent on its
own, but together with the following patches. Its role is to set stage.

dcb_app_print_filtered() should first stop assuming the callback prints
a colon. So you have one patch where the colon moves from the callback
to _print_filtered(). That's a clean, behavior-neutral change that
should be trivial to review. Then next patch introduces the callback.
Which at that point should likewise be tidy, focused and easy to review,
because it will only deal with the new callback.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ