bug-bash
[Top][All Lists]
Advanced

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

Bash 2.05 'unsigned char' cleanup


From: Paul Eggert
Subject: Bash 2.05 'unsigned char' cleanup
Date: Mon, 7 May 2001 13:03:15 -0700 (PDT)

Configuration Information [Automatically generated, do not change]:
Machine: sparc
OS: solaris2.7
Compiler: gcc
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='sparc' 
-DCONF_OSTYPE='solaris2.7' -DCONF_MACHTYPE='sparc-sun-solaris2.7' 
-DCONF_VENDOR='sun' -DSHELL  -DHAVE_CONFIG_H   -I.  -I. -I./include -I./lib 
-I/opt/reb/include -g -O2
uname output: SunOS shade.twinsun.com 5.8 Generic_108528-07 sun4u sparc 
SUNW,Ultra-1
Machine Type: sparc-sun-solaris2.7

Bash Version: 2.05
Patch Level: 0
Release Status: release

Description:
        By manual inspection, I found several places where Bash uses
        'unsigned char' where it's cleaner to use 'char' instead.
        In some places, this violates the C standard; in others, it
        just leads to code that is correct but is error-prone.

        While fixing this, I found one related bug: Bash overruns an
        input buffer while reading the first few bytes of a file to
        see whether it is an executable file.  For example, if the
        file begins with '#', then '!', then 78 spaces, Bash will read
        those 80 bytes into a buffer and then will read past the end
        of the buffer.  Usually this is not a problem in practice, but
        I suppose it might lead to a core dump.

        The enclosed patch fixes both problems.  The execute_cmd.c
        part is the most important one, as it fixes the buffer problem.

Repeat-By:

Fix:

2001-05-07  Paul Eggert  <eggert@twinsun.com>

        Code cleanup spurred by the desire to remove casts to unsigned
        char* or to unsigned char.  This also fixes a read-overrun bug
        with files beginning with '#!' followed by lots of spaces.

        * general.c, general.h (check_binary_file):
        Arg is now char *, not unsigned char *.  All callers changed.

        * execute_cmd.c (execute_shell_script, READ_SAMPLE_BUF): Likewise.
        (execute_shell_script): Arg must be null-terminated.
        All callers changed.
        (STRINGCHAR): Use null-termination rather than length.
        (WHITECHAR): Remove.

        * input.c (localbuf): Now char, not unsigned char.  This fixes
        a technical violation of POSIX, which requires a char * argument
        to `read'.
        (getc_with_restart, ungetc_with_restart): Do not assume that
        localbuf is unsigned char.

        * lib/sh/strtrans.c (ansic_quote):
        Rewrite to avoid cast to unsigned char *.
        * mksyntax.c (addcstr): Likewise.
        * parse.y (yy_string_get): Likewise.

        * lib/sh/zwrite.c (zwrite): Arg is char *, not unsigned char *.
        This fixes a technical violation of the C standard, which does
        not allow you to declare an old-style arg using 'unsigned char
        *', while using a prototype (in another module) using 'char *'.

        * parse.y (yy_readline_get, shell_getc):
        Rewrite to avoid cast to unsigned char.
        * subst.c (expand_word_internal): Likewise.

===================================================================
RCS file: builtins/evalfile.c,v
retrieving revision 2.5.0.1
retrieving revision 2.5.0.2
diff -pu -r2.5.0.1 -r2.5.0.2
--- builtins/evalfile.c 2001/05/03 19:02:50     2.5.0.1
+++ builtins/evalfile.c 2001/05/07 17:56:34     2.5.0.2
@@ -143,7 +143,7 @@ file_error_and_exit:
     }
       
   if ((flags & FEVAL_CHECKBINARY) && 
-      check_binary_file ((unsigned char *)string, (result > 80) ? 80 : result))
+      check_binary_file (string, (result > 80) ? 80 : result))
     {
       free (string);
       (*errfunc) ("%s: cannot execute binary file", filename);
===================================================================
RCS file: execute_cmd.c,v
retrieving revision 2.5.0.7
retrieving revision 2.5.0.8
diff -pu -r2.5.0.7 -r2.5.0.8
--- execute_cmd.c       2001/05/03 19:06:02     2.5.0.7
+++ execute_cmd.c       2001/05/07 19:44:31     2.5.0.8
@@ -3233,6 +3233,8 @@ execute_disk_command (words, redirects, 
    become arguments to the specified interpreter.  ENV is the environment
    to pass to the interpreter.
 
+   SAMPLE[SAMPLE_LEN] must be zero.
+
    The word immediately following the #! is the interpreter to execute.
    A single argument to the interpreter is allowed. */
 
@@ -3243,19 +3245,16 @@ execute_disk_command (words, redirects, 
 
 #if !defined (MSDOS)
 #  define STRINGCHAR(ind) \
-    (!whitespace (sample[ind]) && sample[ind] != '\n' && ind < sample_len)
-#  define WHITECHAR(ind) \
-    (whitespace (sample[ind]) && sample[ind] != '\n' && ind < sample_len)
+    (!whitespace (sample[ind]) && sample[ind] != '\n' && sample[ind])
 #else  /* MSDOS */
 #  define STRINGCHAR(ind) \
-    (!whitespace (sample[ind]) && sample[ind] != '\n' && sample[ind] != '\r' 
&& ind < sample_len)
-#  define WHITECHAR(ind) \
-    (whitespace (sample[ind]) && sample[ind] != '\n' && sample[ind] != '\r' && 
ind < sample_len)
+    (!whitespace (sample[ind]) && sample[ind] != '\n' && sample[ind] \
+     && sample[ind] != '\r')
 #endif /* MSDOS */
 
 static int
 execute_shell_script (sample, sample_len, command, args, env)
-     unsigned char *sample;
+     char *sample;
      int sample_len;
      char *command;
      char **args, **env;
@@ -3265,17 +3264,17 @@ execute_shell_script (sample, sample_len
   int start, size_increment, larry;
 
   /* Find the name of the interpreter to exec. */
-  for (i = 2; whitespace (sample[i]) && i < sample_len; i++)
+  for (i = 2; whitespace (sample[i]); i++)
     ;
 
   for (start = i; STRINGCHAR(i); i++)
     ;
 
-  execname = substring ((char *)sample, start, i);
+  execname = substring (sample, start, i);
   size_increment = 1;
 
   /* Now the argument, if any. */
-  for (firstarg = (char *)NULL, start = i; WHITECHAR(i); i++)
+  for (firstarg = (char *) NULL, start = i; whitespace (sample[i]); i++)
     ;
 
   /* If there is more text on the line, then it is an argument for the
@@ -3285,7 +3284,7 @@ execute_shell_script (sample, sample_len
     {
       for (start = i; STRINGCHAR(i); i++)
        ;
-      firstarg = substring ((char *)sample, start, i);
+      firstarg = substring (sample, start, i);
       size_increment = 2;
     }
 
@@ -3310,7 +3309,6 @@ execute_shell_script (sample, sample_len
   return (shell_execve (execname, args, env));
 }
 #undef STRINGCHAR
-#undef WHITECHAR
 
 #endif /* !HAVE_HASH_BANG_EXEC */
 
@@ -3368,7 +3366,7 @@ initialize_subshell ()
       fd = open(file, O_RDONLY); \
       if (fd >= 0) \
        { \
-         len = read (fd, (char *)buf, 80); \
+         len = read (fd, buf, sizeof buf - 1); \
          close (fd); \
        } \
       else \
@@ -3385,7 +3383,7 @@ shell_execve (command, args, env)
 {
   struct stat finfo;
   int larray, i, fd;
-  unsigned char sample[80];
+  char sample[81];
   int sample_len;
 
   SETOSTYPE (0);               /* Some systems use for USG/POSIX semantics */
@@ -3437,7 +3435,10 @@ shell_execve (command, args, env)
     {
 #if !defined (HAVE_HASH_BANG_EXEC)
       if (sample_len > 2 && sample[0] == '#' && sample[1] == '!')
-       return (execute_shell_script (sample, sample_len, command, args, env));
+       {
+         sample[sample_len] = '\0';
+         return (execute_shell_script (sample, sample_len, command, args, 
env));
+       }
       else
 #endif
       if (check_binary_file (sample, sample_len))
===================================================================
RCS file: general.c,v
retrieving revision 2.5.0.4
retrieving revision 2.5.0.5
diff -pu -r2.5.0.4 -r2.5.0.5
--- general.c   2001/05/01 17:16:04     2.5.0.4
+++ general.c   2001/05/07 19:59:02     2.5.0.5
@@ -393,17 +393,20 @@ move_to_high_fd (fd, check_new, maxfd)
 
 int
 check_binary_file (sample, sample_len)
-     unsigned char *sample;
+     char *sample;
      int sample_len;
 {
   register int i;
+  unsigned char c;
 
   for (i = 0; i < sample_len; i++)
     {
-      if (sample[i] == '\n')
+      c = sample[i];
+
+      if (c == '\n')
        return (0);
 
-      if (isspace (sample[i]) == 0 && isprint (sample[i]) == 0)
+      if (!isspace (c) && !isprint (c))
        return (1);
     }
 
===================================================================
RCS file: general.h,v
retrieving revision 2.5.0.2
retrieving revision 2.5.0.3
diff -pu -r2.5.0.2 -r2.5.0.3
--- general.h   2001/05/01 17:14:41     2.5.0.2
+++ general.h   2001/05/07 17:56:34     2.5.0.3
@@ -241,7 +241,7 @@ extern int sh_unset_nodelay_mode __P((in
 extern void check_dev_tty __P((void));
 extern int same_file ();       /* too many problems with prototype */
 extern int move_to_high_fd __P((int, int, int));
-extern int check_binary_file __P((unsigned char *, int));
+extern int check_binary_file __P((char *, int));
 
 extern char *make_absolute __P((char *, char *));
 extern int absolute_pathname __P((char *));
===================================================================
RCS file: input.c,v
retrieving revision 2.5.0.1
retrieving revision 2.5.0.2
diff -pu -r2.5.0.1 -r2.5.0.2
--- input.c     2001/04/13 08:30:59     2.5.0.1
+++ input.c     2001/05/07 18:15:11     2.5.0.2
@@ -47,7 +47,7 @@ extern int errno;
 /* Functions to handle reading input on systems that don't restart read(2)
    if a signal is received. */
 
-static unsigned char localbuf[128];
+static char localbuf[128];
 static int local_index, local_bufused;
 
 /* Posix and USG systems do not guarantee to restart read () if it is
@@ -57,6 +57,7 @@ int
 getc_with_restart (stream)
      FILE *stream;
 {
+  unsigned char uc;
   /* Try local buffering to reduce the number of read(2) calls. */
   if (local_index == local_bufused || local_bufused == 0)
     {
@@ -73,7 +74,8 @@ getc_with_restart (stream)
        }
       local_index = 0;
     }
-  return (localbuf[local_index++]);
+  uc = localbuf[local_index++];
+  return uc;
 }
 
 int
@@ -83,7 +85,8 @@ ungetc_with_restart (c, stream)
 {
   if (local_index == 0 || c == EOF)
     return EOF;
-  return (localbuf[--local_index] = c);
+  localbuf[--local_index] = c;
+  return c;
 }
 
 #if defined (BUFFERED_INPUT)
===================================================================
RCS file: lib/sh/strtrans.c,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- lib/sh/strtrans.c   2001/02/14 22:03:32     2.5
+++ lib/sh/strtrans.c   2001/05/07 17:56:34     2.5.0.1
@@ -143,7 +143,8 @@ ansic_quote (str, flags, rlen)
      int flags, *rlen;
 {
   char *r, *ret, *s, obuf[8];
-  int l, c, rsize, t;
+  int l, rsize, t;
+  unsigned char c;
 
   if (str == 0 || *str == 0)
     return ((char *)0);
@@ -157,7 +158,7 @@ ansic_quote (str, flags, rlen)
 
   for (s = str, l = 0; *s; s++)
     {
-      c = *(unsigned char *)s;
+      c = *s;
       l = 1;           /* 1 == add backslash; 0 == no backslash */
       switch (c)
        {
===================================================================
RCS file: lib/sh/zwrite.c,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- lib/sh/zwrite.c     2000/10/31 15:52:03     2.5
+++ lib/sh/zwrite.c     2001/05/07 17:56:34     2.5.0.1
@@ -36,7 +36,7 @@ extern int errno;
 int
 zwrite (fd, buf, nb)
      int fd;
-     unsigned char *buf;
+     char *buf;
      size_t nb;
 {
   int n, i, nt;
===================================================================
RCS file: mksyntax.c,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- mksyntax.c  2001/02/14 21:59:25     2.5
+++ mksyntax.c  2001/05/07 17:56:34     2.5.0.1
@@ -150,18 +150,21 @@ addcstr (str, flag)
      char *str;
      int flag;
 {
-  unsigned char *s;
+  char *s;
   char *fstr;
+  unsigned char c;
 
-  for (s = (unsigned char *)str; s && *s; s++)
+  for (s = str; s && *s; s++)
     {
+      c = *s;
+
       if (debug)
        {
          fstr = getcstr (flag);
-         fprintf(stderr, "added %s for character %s\n", fstr, cdesc(*s));
+         fprintf (stderr, "added %s for character %s\n", fstr, cdesc (c));
        }
        
-      lsyntax[*s] |= flag;
+      lsyntax[c] |= flag;
     }
 }
 
===================================================================
RCS file: parse.y,v
retrieving revision 2.5.0.4
retrieving revision 2.5.0.5
diff -pu -r2.5.0.4 -r2.5.0.5
--- parse.y     2001/05/04 00:47:15     2.5.0.4
+++ parse.y     2001/05/07 17:56:34     2.5.0.5
@@ -970,7 +970,8 @@ static int
 yy_readline_get ()
 {
   SigHandler *old_sigint;
-  int line_len, c;
+  int line_len;
+  unsigned char c;
 
   if (!current_readline_line)
     {
@@ -1021,7 +1022,7 @@ yy_readline_get ()
     }
   else
     {
-      c = (unsigned char)current_readline_line[current_readline_line_index++];
+      c = current_readline_line[current_readline_line_index++];
       return (c);
     }
 }
@@ -1067,18 +1068,19 @@ static int
 yy_string_get ()
 {
   register char *string;
-  register int c;
+  register unsigned char c;
 
   string = bash_input.location.string;
-  c = EOF;
 
   /* If the string doesn't exist, or is empty, EOF found. */
   if (string && *string)
     {
-      c = *(unsigned char *)string++;
+      c = *string++;
       bash_input.location.string = string;
+      return (c);
     }
-  return (c);
+  else
+    return EOF;
 }
 
 static int
@@ -1585,6 +1587,7 @@ shell_getc (remove_quoted_newline)
 {
   register int i;
   int c;
+  unsigned char uc;
   static int mustpop = 0;
 
   QUIT;
@@ -1763,12 +1766,12 @@ shell_getc (remove_quoted_newline)
        }
     }
 
-  c = shell_input_line[shell_input_line_index];
+  uc = shell_input_line[shell_input_line_index];
 
-  if (c)
+  if (uc)
     shell_input_line_index++;
 
-  if (c == '\\' && remove_quoted_newline &&
+  if (uc == '\\' && remove_quoted_newline &&
       shell_input_line[shell_input_line_index] == '\n')
     {
        prompt_again ();
@@ -1777,33 +1780,33 @@ shell_getc (remove_quoted_newline)
     }
 
 #if defined (ALIAS) || defined (DPAREN_ARITHMETIC)
-  /* If C is NULL, we have reached the end of the current input string.  If
+  /* If UC is NULL, we have reached the end of the current input string.  If
      pushed_string_list is non-empty, it's time to pop to the previous string
      because we have fully consumed the result of the last alias expansion.
      Do it transparently; just return the next character of the string popped
      to. */
-  if (!c && (pushed_string_list != (STRING_SAVER *)NULL))
+  if (!uc && (pushed_string_list != (STRING_SAVER *)NULL))
     {
       if (mustpop)
        {
          pop_string ();
-         c = shell_input_line[shell_input_line_index];
-         if (c)
+         uc = shell_input_line[shell_input_line_index];
+         if (uc)
            shell_input_line_index++;
          mustpop--;
        }
       else
        {
          mustpop++;
-         c = ' ';
+         uc = ' ';
        }
     }
 #endif /* ALIAS || DPAREN_ARITHMETIC */
 
-  if (!c && shell_input_line_terminator == EOF)
+  if (!uc && shell_input_line_terminator == EOF)
     return ((shell_input_line_index != 0) ? '\n' : EOF);
 
-  return ((unsigned char)c);
+  return (uc);
 }
 
 /* Put C back into the input for the shell. */
===================================================================
RCS file: shell.c,v
retrieving revision 2.5
retrieving revision 2.5.0.1
diff -pu -r2.5 -r2.5.0.1
--- shell.c     2001/03/27 14:25:51     2.5
+++ shell.c     2001/05/07 17:56:34     2.5.0.1
@@ -1198,7 +1198,7 @@ open_shell_script (script_name)
 {
   int fd, e, fd_is_tty;
   char *filename, *path_filename;
-  unsigned char sample[80];
+  char sample[80];
   int sample_len;
   struct stat sb;
 
===================================================================
RCS file: subst.c,v
retrieving revision 2.5.0.9
retrieving revision 2.5.0.10
diff -pu -r2.5.0.9 -r2.5.0.10
--- subst.c     2001/05/03 20:13:05     2.5.0.9
+++ subst.c     2001/05/07 17:56:34     2.5.0.10
@@ -5355,6 +5355,7 @@ expand_word_internal (word, quoted, isex
   int tflag;
 
   register int c;              /* Current character. */
+  unsigned char uc;
   int t_index;                 /* For calls to string_extract_xxx. */
 
   char ifscmap[256];
@@ -5384,7 +5385,10 @@ expand_word_internal (word, quoted, isex
        ${abc:-G { I } K } as one word when it should be three. */
     if (*temp1 != ' ' && *temp1 != '\t' && *temp1 != '\n')
 #endif
-      ifscmap[(unsigned char)*temp1] = 1;
+      {
+       uc = *temp1;
+       ifscmap[uc] = 1;
+      }
 
   /* Begin the expansion. */
 



reply via email to

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