linux-kernel - Re: [PATCH] staging: s5k3e2fx.c: simplify complexity by factoring

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, 18 Dec 2009 15:52:53 -0800
From:	Justin Madru <jdm64@...ab.com>
To:	Greg KH <greg@...ah.com>
CC:	linux-kernel@...r.kernel.org, gregkh@...e.de, error27@...il.com,
	ray-lk@...rabbit.org
Subject: Re: [PATCH] staging: s5k3e2fx.c: simplify complexity by factoring

On 12/18/2009 10:41 AM, Greg KH wrote:

> On Tue, Dec 15, 2009 at 12:52:10PM -0800, Justin Madru wrote:
>> the code was looping, seting s_move[i] to the following calculations
>>
>> if (actual_step>= 0)
>>          s_move[i] = ((((i + 1) * gain + 0x200) - (i * gain + 0x200)) / 0x400);
>> else
>>          s_move[i] = ((((i + 1) * gain - 0x200) - (i * gain - 0x200)) / 0x400);
>>
>> but, this code redues to the expression
>> 	s_move[i] = gain>>  10;
>>
>> The reason for the complexity was to generate a step function with
>> integer division and rounding to land on specific values. But these calculations
>> can be simplified to the following code:
>>
>> 	gain = ((actual_step<<  10) / 5)>>  10;
>> 	for (i = 0; i<= 4; i++)
>> 		s_move[i] = gain;
>
> Care to resend this with a Signed-off-by: line, and add the reviewed-by
> line that was requested by Ray?
>
> thanks,
>
> greg k-h
Sorry about that, new at submitting patches, don't completely know the flow yet.
Does the below format work for you? Hopefully my email client won't mess the patch up.

Justin Madru
------

From: Justin Madru<jdm64@...ab.com>
Date: Tue, 15 Dec 2009 12:27:31 -0800
Subject: [PATCH] staging: s5k3e2fx.c: simplify complexity by factoring

the code was looping, setting s_move[i] to the following calculations

if (actual_step>= 0)
         s_move[i] = ((((i + 1) * gain + 0x200) - (i * gain + 0x200)) / 0x400);
else
         s_move[i] = ((((i + 1) * gain - 0x200) - (i * gain - 0x200)) / 0x400);

but, this code reduces to the expression
	s_move[i] = gain>>  10;

The reason for the complexity was to generate a step function with
integer division and rounding to land on specific values. But these calculations
can be simplified to the following code:

	gain = ((actual_step<<  10) / 5)>>  10;
	for (i = 0; i<= 4; i++)
		s_move[i] = gain;

Signed-off-by: Justin Madru<jdm64@...ab.com>
Reviewed-by: Ray Lee<ray-lk@...rabbit.org>
---
  drivers/staging/dream/camera/s5k3e2fx.c |   10 +++-------
  1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/dream/camera/s5k3e2fx.c b/drivers/staging/dream/camera/s5k3e2fx.c
index f0e49be..93162a0 100644
--- a/drivers/staging/dream/camera/s5k3e2fx.c
+++ b/drivers/staging/dream/camera/s5k3e2fx.c
@@ -1092,14 +1092,10 @@ static int32_t s5k3e2fx_move_focus(int direction, int32_t num_steps)

  	actual_step = step_direction * (int16_t)num_steps;
  	pos_offset = init_code + s5k3e2fx_ctrl->curr_lens_pos;
-	gain = actual_step * 0x400 / 5;
+	gain = ((actual_step<<  10) / 5)>>  10;

-	for (i = 0; i<= 4; i++) {
-		if (actual_step>= 0)
-			s_move[i] = ((((i+1)*gain+0x200) - (i*gain+0x200))/0x400);
-		else
-			s_move[i] = ((((i+1)*gain-0x200) - (i*gain-0x200))/0x400);
-	}
+	for (i = 0; i<= 4; i++)
+		s_move[i] = gain;

  	/* Ring Damping Code */
  	for (i = 0; i<= 4; i++) {
-- 
1.6.5.6


--
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/