linux-kernel - Re: [PATCH] reset: generate reset_control

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] [day] [month] [year] [list]

Date:	Fri, 22 Jul 2016 14:58:04 +0200
From:	Philipp Zabel <p.zabel@...gutronix.de>
To:	Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Hans de Goede <hdegoede@...hat.com>,
	Lee Jones <lee.jones@...aro.org>
Subject: Re: [PATCH] reset: generate reset_control_get variants with macro
 expansion

Hi Masahiro,

Am Donnerstag, den 21.07.2016, 18:53 +0900 schrieb Masahiro Yamada:
[...]
> For ctags, scripts/tags.sh exists for that purpose, doesn't it?
> 
> For example,
> commit e0e5070b20e01f0321f97db4e4e174f3f6b49e50
> converted func defines and adjusted tags.sh at the same time.

True. So ctags is covered, alhough this somewhat moves the maintenance
effort from nearly duplicated function definitions to keeping
scripts/tags.sh up to date with all the different macro function
generators instead.

> > And secondly, we would now have detailed kerneldoc comments for two
> > functions that are never called directly, but the public facing API
> > itself is completely without documentation. I wouldn't mind removing
> > duplicated documentation paragraphs though, for example by referencing
> > reset_control_get_* from the of_reset_control_get_* kerneldoc comments.
> 
> Linux is not always kind enough to document every public API.
> 
> As you see, we generally do not document
> static inline wrappers.

Why not? As long as there is a meaningful difference between them, it
would make sense to describe them individually.

> In our case, they are all simply derived from two base functions.
> Documenting the two is enough.

I disagree. As a user of an API and its documentation, I would like to
have descriptions of functions that are actually used in the code, not
descriptions of the internal implementation behind them. This also
allows to have shorter paragraphs for each function, since they only
have to describe the behavior of that particular variant.

> I need to expand drivers/usb/host/ehci-platform.c
> to support multiple reset lines, so I really want
> devm_reset_control_get_optional_shared_by_index() supported.
> 
> (So, probably
> devm_reset_control_get_exclusive_by_index
> devm_reset_control_get_shared_by_index
> devm_reset_control_get_optional_exclusive_by_index
> as well)
>
> If you are unhappy with this patch,
> I will send an alternative one,
> which adds 4 more functions in a stupid and straightforward way.

Yes, I would prefer straightforward addition of the needed inline
functions.

regards
Philipp