linux-kernel - Re: [lm-sensors] [PATCH 1/4] kernel.h: add find

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAMpxmJVUcuyLJ9ju0U8kMg6AEFYijF8=_yHgYF7EMV63qbLCmg@mail.gmail.com>
Date:	Wed, 25 Feb 2015 11:59:51 +0100
From:	Bartosz Golaszewski <bgolaszewski@...libre.com>
To:	Guenter Roeck <linux@...ck-us.net>
Cc:	Phil Pokorny <ppokorny@...guincomputing.com>,
	LKML <linux-kernel@...r.kernel.org>,
	lm-sensors <lm-sensors@...sensors.org>
Subject: Re: [lm-sensors] [PATCH 1/4] kernel.h: add find_closest() macro

2015-02-24 21:51 GMT+01:00 Guenter Roeck <linux@...ck-us.net>:
> I think the lm85 conversion actually introduces a bug with such an
> off-by-one mistake. And if it doesn't, there is still a unexplained
> and not easy to understand '-1' in one of the calls to find_closest().
>
> So the question is if the new code really improves the situation in that
> respect.

Yes, it's a mistake. I couldn't test this one and missed this '-1'.
Sorry for that.

As for the size comparisons - at first glance it seems as if it adds bloat:

ina2xx:
add/remove: 0/0 grow/shrink: 1/0 up/down: 24/0 (24)
function                                     old     new   delta
ina226_set_interval                          296     320     +24

lm85:
add/remove: 0/0 grow/shrink: 3/0 up/down: 72/0 (72)
function                                     old     new   delta
set_temp_auto_temp_min                       364     388     +24
set_temp_auto_temp_max                       336     360     +24
set_pwm_freq                                 284     308     +24

w83795:
add/remove: 0/0 grow/shrink: 1/0 up/down: 16/0 (16)
function                                     old     new   delta
store_pwm                                    496     512     +16

But this actually comes from DIV_ROUND_CLOSEST() since replacing it
with a simple '/ 2' gives different results:

ina2xx.ko:
add/remove: 0/0 grow/shrink: 1/0 up/down: 6/0 (6)
function                                     old     new   delta
__UNIQUE_ID_vermagic0                         73      79      +6

lm85.ko:
add/remove: 0/0 grow/shrink: 1/0 up/down: 6/0 (6)
function                                     old     new   delta
__UNIQUE_ID_vermagic0                         73      79      +6

w83795.ko:
add/remove: 0/0 grow/shrink: 2/0 up/down: 14/0 (14)
function                                     old     new   delta
store_pwm                                    496     504      +8
__UNIQUE_ID_vermagic0                         73      79      +6

The idea however was to remove duplicated operations from source files
and prevent off-by-one bugs (how ironic the lm85 patch...), not really
to reduce size.

Bart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/