Hello all,
David Pirotte has been experiencing a problem where (reload-module ...)
was failing to trigger a recompilation even after the source file has
been modified. We did some debugging, and discovered that when
'compiled_is_fresh' is called during the failed reload, the
'stat_source' structure contains garbage.
My investigation revealed a flaw in 'search_path'. Although it is not
mentioned in the header comment, it is clearly supposed to fill the
'stat_buf' whenever it is successful (scm_primitive_load_path assumes
that it will, anyway).
The problem is that there's a case when 'search_path' leaves the
'stat_buf' uninitialized. If the provided 'filename' is an absolute
pathname, then it simply returns this pathname unchanged without
touching the 'stat_buf'. This is bad, because 'scm_primitive_load_path'
proceeds to pass this uninitialized 'stat_buf' to 'compiled_is_fresh'.
I've attached a patch to (hopefully) fix this problem. Unfortunately,
David tells me that this doesn't solve his reload issue, so unless he
made a mistake in testing it, I guess there's another bug still lurking,
or perhaps this fix is incorrect.
Reviews solicited.
Thanks,
Mark
>From b3e8ae048d87ab28c90b1c1c635a5116ee6f080c Mon Sep 17 00:00:00 2001
From: Mark H Weaver <m...@netris.org>
Date: Wed, 1 Feb 2012 16:35:32 -0500
Subject: [PATCH] Fix search_path to fill stat_buf when given an absolute
pathname
* libguile/load.c (search_path): When the provided 'filename' is an
absolute pathname, perform a 'stat' on that pathname to fill the
'stat_buf'. Previously, 'stat_buf' was left uninitialized in this
case, even though 'scm_primitive_load_path' assumes that 'stat_buf'
will be filled. Update the header comment to explicitly specify that
'stat_buf' will be filled. Also 'goto end' in a few failure cases
instead of replicating its code.
---
libguile/load.c | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/libguile/load.c b/libguile/load.c
index 0076218..ae9e4bd 100644
--- a/libguile/load.c
+++ b/libguile/load.c
@@ -419,8 +419,9 @@ scm_c_string_has_an_ext (char *str, size_t len, SCM extensions)
/* Search PATH for a directory containing a file named FILENAME.
The file must be readable, and not a directory.
- If we find one, return its full filename; otherwise, return #f.
+ If we find one, return its full pathname; otherwise, return #f.
If FILENAME is absolute, return it unchanged.
+ We also fill *stat_buf corresponding to the returned pathname.
If given, EXTENSIONS is a list of strings; for each directory
in PATH, we search for FILENAME concatenated with each EXTENSION. */
static SCM
@@ -445,7 +446,7 @@ search_path (SCM path, SCM filename, SCM extensions, SCM require_exts,
filename_len = strlen (filename_chars);
scm_dynwind_free (filename_chars);
- /* If FILENAME is absolute, return it unchanged. */
+ /* If FILENAME is absolute and is still valid, return it unchanged. */
#ifdef __MINGW32__
if (((filename_len >= 1) &&
(filename_chars[0] == '/' || filename_chars[0] == '\\')) ||
@@ -457,14 +458,13 @@ search_path (SCM path, SCM filename, SCM extensions, SCM require_exts,
if (filename_len >= 1 && filename_chars[0] == '/')
#endif
{
- SCM res = filename;
- if (scm_is_true (require_exts) &&
- !scm_c_string_has_an_ext (filename_chars, filename_len,
+ if ((scm_is_false (require_exts) ||
+ scm_c_string_has_an_ext (filename_chars, filename_len,
extensions))
- res = SCM_BOOL_F;
-
- scm_dynwind_end ();
- return res;
+ && stat (filename_chars, stat_buf) == 0
+ && !(stat_buf->st_mode & S_IFDIR))
+ result = filename;
+ goto end;
}
/* If FILENAME has an extension, don't try to add EXTENSIONS to it. */
@@ -483,8 +483,7 @@ search_path (SCM path, SCM filename, SCM extensions, SCM require_exts,
{
/* This filename has an extension, but not one of the right
ones... */
- scm_dynwind_end ();
- return SCM_BOOL_F;
+ goto end;
}
/* This filename already has an extension, so cancel the
list of extensions. */
--
1.7.5.4
