Skip to contentSkip to conversation

Fix integer overflow warning in calc_divisor()

PATCH New Hugo Villeneuve 3 participants 6 messages Jul 11, 2008 – Jul 15, 2008

lore
Share & tools
lore

Hugo Villeneuve

author
#0 lore raw

PATCH 1/1 Fix integer overflow warning in calc_divisor()

Fix the integer overflow warning when rounding the serial port clock divisor value in calc_divisor().

Signed-off-by Hugo Villeneuve <[email protected]>

---

 drivers/serial/serial.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)
drivers/serial/serial.c +9 -2
index 182ca2d..4ccaee2 100644
@@ -124,6 +124,8 @@ static NS16550_t serial_ports[4] = {
 
 static int calc_divisor (NS16550_t port)
 {
+	uint32_t clk_divisor;
+
 #ifdef CONFIG_OMAP1510
 	/* If can't cleanly clock 115200 set div to 1 */
 	if ((CFG_NS16550_CLK == 12000000) && (gd->baudrate == 115200)) {

@@ -147,10 +149,15 @@ static int calc_divisor (NS16550_t port)
 
 	/* Compute divisor value. Normally, we should simply return:
 	 *   CFG_NS16550_CLK) / MODE_X_DIV / gd->baudrate
-	 * but we need to round that value by adding 0.5 or 8/16.
+	 * but we need to round that value by adding 0.5 (2/4).
 	 * Rounding is especially important at high baud rates.
 	 */
-	return (((16 * CFG_NS16550_CLK) / MODE_X_DIV / gd->baudrate) + 8) / 16;
+	clk_divisor = (((4 * CFG_NS16550_CLK) /
+			(MODE_X_DIV * gd->baudrate)) + 2) / 4;
+
+	debug("NS16550 clock divisor = %d\n", clk_divisor);
+
+	return clk_divisor;
 }
 
 #if !defined(CONFIG_SERIAL_MULTI)
Applied
#1 lore raw

PATCH 1/1 Fix integer overflow warning in calc_divisor()

Quoted 9 lines · In message <[email protected]> you…

Fix the integer overflow warning when rounding the serial port clock divisor value in calc_divisor().

Signed-off-by Hugo Villeneuve <[email protected]>

---

 drivers/serial/serial.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

Applied, thanks, but I really wish you had kept the subject unchanged, and ideally even threading correct.

Best regards,

Wolfgang Denk

Signature · 4 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In C we had to code our own bugs, in C++ we can inherit them.
author
#2 lore raw

PATCH 1/1 Fix integer overflow warning in calc_divisor()

Quoted 6 lines · Wolfgang Denk
Quoted 5 lines · In message <[email protected]> you…

Fix the integer overflow warning when rounding the serial port clock divisor value in calc_divisor().

Signed-off-by Hugo Villeneuve <[email protected]>
Quoted 7 lines
Quoted 4 lines

---

 drivers/serial/serial.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

Applied, thanks, but I really wish you had kept the subject unchanged, and ideally even threading correct.

Hi Wolfgang,
since you already applied the first patch, I simply thought that the fix for the first patch should have a different subject.

I will keep the same subject in the future

Hugo V.

#3 lore raw

PATCH 1/1 Fix integer overflow warning in calc_divisor()

Quoted 5 lines · In message <[email protected]> you wrote:

since you already applied the first patch, I simply thought that the fix for the first patch should have a different subject.

I will keep the same subject in the future

Thanks. You have another try anyway ;-)

The bug is not fixed yet:

Configuring for mpc7448hpc2 board... serial.c: In function 'calc_divisor':
serial.c:155: warning: integer overflow in expression

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Anarchy may not be the best form of government, but it's better  than
no government at all.
#4 lore raw

PATCH v2 1/1 Round the serial port clock divisor value returned by calc_divisor()

This formula is better at avoiding integer overflow.

Signed-off-by Gerald Van Baren <[email protected]>

---

This is my latest entry in the baud rate rounding dual. Since it doesn't multiply the master BRG clock but instead adds the baud rate scaled by 1/2 the clock multiplier, it should not overflow (for a master clock right at the edge of overflowing itself, it still will overflow, but that is pretty unlikely).

This compiles OK on the mpc7448hpc2. I have only tested it on a calculator. I have NOT tested it on real hardware.

 drivers/serial/serial.c |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)
drivers/serial/serial.c +5 -11
index 4ccaee2..7f43540 100644
@@ -124,7 +124,6 @@ static NS16550_t serial_ports[4] = {
 
 static int calc_divisor (NS16550_t port)
 {
-	uint32_t clk_divisor;
 
 #ifdef CONFIG_OMAP1510
 	/* If can't cleanly clock 115200 set div to 1 */

@@ -147,17 +146,12 @@ static int calc_divisor (NS16550_t port)
 #define MODE_X_DIV 16
 #endif
 
-	/* Compute divisor value. Normally, we should simply return:
-	 *   CFG_NS16550_CLK) / MODE_X_DIV / gd->baudrate
-	 * but we need to round that value by adding 0.5 (2/4).
-	 * Rounding is especially important at high baud rates.
+	/*
+	 * Compute divisor value, rounding it properly.  Rounding is
+	 * especially important at high baud rates.
 	 */
-	clk_divisor = (((4 * CFG_NS16550_CLK) /
-			(MODE_X_DIV * gd->baudrate)) + 2) / 4;
-
-	debug("NS16550 clock divisor = %d\n", clk_divisor);
-
-	return clk_divisor;
+	return (CFG_NS16550_CLK + (gd->baudrate * (MODE_X_DIV / 2))) /
+		(MODE_X_DIV * gd->baudrate);
 }
 
 #if !defined(CONFIG_SERIAL_MULTI)
Signature · 1 line
1.5.6
author
#5 lore raw

PATCH v2 1/1 Round the serial port clock divisor valuereturned by calc_divisor()

Quoted 5 lines · u-boot-users-bounces at lists.sourceforge.net

This formula is better at avoiding integer overflow.

Signed-off-by Gerald Van Baren <[email protected]>

---

Quoted 8 lines

This is my latest entry in the baud rate rounding dual. Since it doesn't multiply the master BRG clock but instead adds the baud rate scaled by 1/2 the clock multiplier, it should not overflow (for a master clock right at the edge of overflowing itself, it still will overflow, but that is pretty unlikely).

This compiles OK on the mpc7448hpc2. I have only tested it on a calculator. I have NOT tested it on real hardware.

I have tested it on my board and it works.

I also tested it on my board by using the clock value of the mpc7448hpc2 board, and printing the computed value which was correct.

drivers/serial/serial.c static int calc_divisor (NS16550_t port) · line 152

Quoted 11 lines
 drivers/serial/serial.c |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

...

drivers/serial/serial.c +3 -4
-	/* Compute divisor value. Normally, we should simply return:
-	 *   CFG_NS16550_CLK) / MODE_X_DIV / gd->baudrate
-	 * but we need to round that value by adding 0.5 (2/4).
-	 * Rounding is especially important at high baud rates.
+	/*
+	 * Compute divisor value, rounding it properly.  Rounding is
+	 * especially important at high baud rates.
 	 */

Please keep the comments explaining the base formula: it is not easy to reverse-engineer an optimized formula to figure out what is the base formula.

I will resubmit a patch with your fix and my comments.

Hugo V.