linux-kernel - Re: [PATCH v2 00/15] Introduce block device LED trigger

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]

Date:   Fri, 10 Sep 2021 04:09:59 +0200
From:   Marek Behún <kabel@...nel.org>
To:     Ian Pilcher <arequipeno@...il.com>
Cc:     axboe@...nel.dk, pavel@....cz, linux-leds@...r.kernel.org,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        gregkh@...uxfoundation.org
Subject: Re: [PATCH v2 00/15] Introduce block device LED trigger

Dear Ian,

I have tried to look into this and replied to some of your patches.

There are still many things to do, and I think the reviewing would be
much easier to review if you sent all the code changes as one patch
(since the changes are doing an atomic change: adding support for blkdev
LED trigger). Keep only the sysfs doc change in a separate patch.

You are unnecessary using the const keyword in places where it is not
needed and not customary for Linux kernel codebase. See in another of
my replies.

You are using a weird comment style, i.e.
  /*
   *
   *	Disassociate an LED from the trigger
   *
   */

  static void blkdev_deactivate(struct led_classdev *const led_dev)

Please look at how functions are documented in led-class.c, for example.

There are many other things I would like you to change and fix,
I will comment on them once you send this proposal as two commits:
one sysfs docs change, one code change.

Marek