This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] windres: Fix read_rc_file and close_input_sream.


This patch fixes the following two issues in windres:

1. In read_rc_file the wrong varaible is passed to 
   windres_add_include_dir 

2. In close_input_stream the return of pclose is ignored.

In read_rc_file "e" was passed to windres_add_include_dir instead of
"c".  The variable "c" contains a directory to search, and "e" is a
pointer to the last strchr replacement. The patch replaces "e" with
"edit" and "c" with "dir" to make the intent of the variables clear. We
correctly passes "dir" to windres_add_include_dir. Comments are added
where appropriate to make the code clearer. The algorithm is not
changed. To prevent this from happening again the patch adds asserts in
windres_add_include_dir to reject invalid paths. Invalid paths should be
rejected by callers to windres_add_include_dir.

In close_input_stream we check the return of pclose to verify that cpp
actually succeeded in preprocessing the .rc file. A non-zero exit status
from cpp should be considered a hard failure, so we call fatal.  With
this chagne you get "preprocessing failed" preceeded by the error
e.g. "./PAGEANT.RC:21:17: error: foo.h: No such file or directory".
Without this change you may get a parser error, as yyparse fails on
tokens which were not preprocessed.

No regressions on i686-mingw32.

OK to checkin?

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
carlos@codesourcery.com
(650) 331-3385 x716

2007-10-05  Carlos O'Donell  <carlos@codesourcery.com>

	* resrc.c (read_rc_file): Rename e to edit, and c to dir.
	Pass dir to windres_add_include_dir.  Add comments.
	(close_input_stream): Check pclose error, and call fatal if
	the preprocessor failed.
	* windres.c (windres_add_include_dir): Assert that p is non-NULL,
	and not an empty string. 

Index: binutils/resrc.c
===================================================================
RCS file: /cvs/src/src/binutils/resrc.c,v
retrieving revision 1.33
diff -u -p -r1.33 resrc.c
--- binutils/resrc.c	17 Sep 2007 09:58:27 -0000	1.33
+++ binutils/resrc.c	5 Oct 2007 15:26:01 -0000
@@ -441,30 +441,37 @@ read_rc_file (const char *filename, cons
   /* Setup the default resource import path taken from input file.  */
   else if (strchr (filename, '/') != NULL || strchr (filename, '\\') != NULL)
     {
-      char *e, *c;
+      char *edit, *dir;
 
       if (filename[0] == '/'
 	  || filename[0] == '\\'
 	  || filename[1] == ':')
-	e = c = xstrdup (filename);
+        /* Absolute path.  */
+	edit = dir = xstrdup (filename);
       else
 	{
-	  e = c = xmalloc (strlen (filename) + 3);
-	  sprintf (c, "./%s", filename);
+	  /* Relative path.  */
+	  edit = dir = xmalloc (strlen (filename) + 3);
+	  sprintf (dir, "./%s", filename);
 	}
-      e += strlen (c);
-      while (e > c && (e[-1] != '\\' && e[-1] != '/'))
+
+      /* Walk dir backwards stopping at the first directory separator.  */
+      edit += strlen (dir);
+      while (edit > dir && (edit[-1] != '\\' && edit[-1] != '/'))
 	{
-	  --e;
-	  e[0] = 0;
+	  --edit;
+	  edit[0] = 0;
 	}
+
       /* Cut off trailing slash.  */
-      --e;
-      e[0] = 0;
-      while ((e = strchr (c, '\\')) != NULL)
-	*e = '/';
+      --edit;
+      edit[0] = 0;
+
+      /* Convert all back slashes to forward slashes.  */
+      while ((edit = strchr (dir, '\\')) != NULL)
+	*edit = '/';
 
-      windres_add_include_dir (e);
+      windres_add_include_dir (dir);
     }
 
   istream_type = (use_temp_file) ? ISTREAM_FILE : ISTREAM_PIPE;
@@ -588,7 +595,19 @@ close_input_stream (void)
   else
     {
       if (cpp_pipe != NULL)
-	pclose (cpp_pipe);
+        {
+	  int err;
+	  err = pclose (cpp_pipe);
+	  /* We are reading from a pipe, therefore we don't
+             know if cpp failed or succeeded until pclose.  */
+	  if (err != 0 || errno == ECHILD)
+	    {
+	      /* Since this is also run via xatexit, safeguard.  */
+	      cpp_pipe = NULL;
+	      cpp_temp_file = NULL;
+	      fatal (_("preprocessing failed."));
+	    }
+        }
     }
 
   /* Since this is also run via xatexit, safeguard.  */
Index: binutils/windres.c
===================================================================
RCS file: /cvs/src/src/binutils/windres.c,v
retrieving revision 1.33
diff -u -p -r1.33 windres.c
--- binutils/windres.c	5 Jul 2007 16:54:45 -0000	1.33
+++ binutils/windres.c	5 Oct 2007 15:26:01 -0000
@@ -765,6 +765,12 @@ windres_add_include_dir (const char *p)
 {
   struct include_dir *n, **pp;
 
+  /* Computing paths is often complicated and error prone.
+     The easiest way to check for mistakes is at the time
+     we add them to include_dirs.  */
+  assert (p != NULL);
+  assert (*p != '\0');
+
   n = xmalloc (sizeof *n);
   n->next = NULL;
   n->dir = (char * ) p;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]