bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] timeout: remove problematic casts


From: Pádraig Brady
Subject: [PATCH] timeout: remove problematic casts
Date: Tue, 23 Dec 2008 09:46:21 +0000
User-agent: Thunderbird 2.0.0.6 (X11/20071008)

There is some dodgy casts in timeout.c that are being
flagged by `gcc -fstrict-aliasing -Wstrict-aliasing`
The attached patch hopefully addresses this.

cheers,
Pádraig.
>From 693467d432a4718068137e97c5655b7b8ff0f86c Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Tue, 23 Dec 2008 09:36:22 +0000
Subject: [PATCH] timeout: remove problematic casts

src/timeout.c (apply_time_suffix): Change input parameter from unsigned int
to unsigned long, which is the type of the variable it actually manipulates.
This removes the need for the cast which was giving a warning with the
gcc options: -fstrict-aliasing -Wstrict-aliasing
Also add a check for overflow possible on 16 bit platforms, and fix indents.
Outside this function a redundant cast is removed in the alarm() call.
---
 src/timeout.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/timeout.c b/src/timeout.c
index e8ecf62..8ef4b54 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -164,17 +164,17 @@ be caught.\n"), stdout);
   exit (status);
 }
 
-/* Given an integer value *X, and a suffix character, SUFFIX_CHAR,
+/* Given a long integer value *X, and a suffix character, SUFFIX_CHAR,
    scale *X by the multiplier implied by SUFFIX_CHAR.  SUFFIX_CHAR may
    be the NUL byte or `s' to denote seconds, `m' for minutes, `h' for
    hours, or `d' for days.  If SUFFIX_CHAR is invalid, don't modify *X
-   and return false.  If *X would overflow, don't modify *X and return false.
-   Otherwise return true.  */
+   and return false.  If *X would overflow an integer, don't modify *X
+   and return false. Otherwise return true.  */
 
 static bool
-apply_time_suffix (unsigned int *x, char suffix_char)
+apply_time_suffix (unsigned long *x, char suffix_char)
 {
-  int multiplier = 1;
+  unsigned int multiplier = 1;
 
   switch (suffix_char)
     {
@@ -186,6 +186,8 @@ apply_time_suffix (unsigned int *x, char suffix_char)
     case 'h':
       multiplier *= 60;
     case 'm':
+      if (multiplier > UINT_MAX / 60) /* 16 bit overflow */
+        return false;
       multiplier *= 60;
       break;
     default:
@@ -193,7 +195,7 @@ apply_time_suffix (unsigned int *x, char suffix_char)
     }
 
   if (*x > UINT_MAX / multiplier)
-      return false;
+    return false;
 
   *x *= multiplier;
 
@@ -259,7 +261,7 @@ main (int argc, char **argv)
       /* Extra chars after the number and an optional s,m,h,d char.  */
       || (*ep && *(ep + 1))
       /* Check any suffix char and update timeout based on the suffix.  */
-      || !apply_time_suffix ((unsigned int *) &timeout, *ep))
+      || !apply_time_suffix (&timeout, *ep))
     {
       error (0, 0, _("invalid time interval %s"), quote (argv[optind]));
       usage (EXIT_CANCELED);
@@ -306,7 +308,7 @@ main (int argc, char **argv)
     {
       int status;
 
-      alarm ((unsigned int) timeout);
+      alarm (timeout);
 
       /* We're just waiting for a single process here, so wait() suffices.
          Note the signal() calls above on linux and BSD at least, essentially
-- 
1.5.3.6


reply via email to

[Prev in Thread] Current Thread [Next in Thread]