R devel - Mar 2007 - Subtle bug in do_basename

Hello,


I've been wondering why my no-optimization R-devel builds have been 
hanging during "building/updating package indices ...". I tracked it 
down with gdb to this line from do_basename in utils.c:

while ( *(p = buf + strlen(buf) - 1) == fsp ) *p = '\0';

Now, imagine if your compiler places the variable fsp immediately before 
buf on the stack, and strlen(buf) is 0. Yup, you get an infinite loop 
because p will always be assigned the address of fsp. I'm not quite sure 
what happens when the stack variables are ordered in a different 
configuration, probably something bad?

Here's a quick fix, but maybe someone would want to find a better one:

$ svn diff src/main/util.c
Index: src/main/util.c
==================================================================---
src/main/util.c     (revision 40876)
+++ src/main/util.c     (working copy)
@@ -694,7 +694,8 @@
         R_fixslash(buf);
  #endif
         /* remove trailing file separator(s) */
-       while ( *(p = buf + strlen(buf) - 1) == fsp ) *p = '\0';
+       if(strlen(p))
+           while ( *(p = buf + strlen(buf) - 1) == fsp ) *p = '\0';
         if ((p = Rf_strrchr(buf, fsp)))
             p++;
         else

Best,

Jeff
-- 
http://biostat.mc.vanderbilt.edu/JeffreyHorner

On 3/24/2007 6:02 PM, Jeffrey Horner wrote:
> Hello,
> 
> 
> I've been wondering why my no-optimization R-devel builds have been 
> hanging during "building/updating package indices ...". I tracked
it
> down with gdb to this line from do_basename in utils.c:
> 
> while ( *(p = buf + strlen(buf) - 1) == fsp ) *p = '\0';
> 
> Now, imagine if your compiler places the variable fsp immediately before 
> buf on the stack, and strlen(buf) is 0. Yup, you get an infinite loop 
> because p will always be assigned the address of fsp. I'm not quite
sure
> what happens when the stack variables are ordered in a different 
> configuration, probably something bad?
> 
> Here's a quick fix, but maybe someone would want to find a better one:
I think that looks like the right solution; I'll commit it.

Duncan Murdoch
> 
> $ svn diff src/main/util.c
> Index: src/main/util.c
> ==================================================================> ---
src/main/util.c     (revision 40876)
> +++ src/main/util.c     (working copy)
> @@ -694,7 +694,8 @@
>          R_fixslash(buf);
>   #endif
>          /* remove trailing file separator(s) */
> -       while ( *(p = buf + strlen(buf) - 1) == fsp ) *p = '\0';
> +       if(strlen(p))
> +           while ( *(p = buf + strlen(buf) - 1) == fsp ) *p =
'\0';
>          if ((p = Rf_strrchr(buf, fsp)))
>              p++;
>          else
> 
> Best,
> 
> Jeff

Good catch, there is even one more problem I believe - it  can still  
run past the beginning of the buffer (e.g. if you use "///").
This is a bit more more efficient due to fewer calls to strlen and  
fixes both problems:

if (*buf) {
   p = buf + strlen(buf) - 1;
   while (p>=buf && *p == fsp) *(p--)='\0';
}

Cheers,
Simon

On Mar 24, 2007, at 6:02 PM, Jeffrey Horner wrote:
> Hello,
>
>
> I've been wondering why my no-optimization R-devel builds have been
> hanging during "building/updating package indices ...". I tracked
it
> down with gdb to this line from do_basename in utils.c:
>
> while ( *(p = buf + strlen(buf) - 1) == fsp ) *p = '\0';
>
> Now, imagine if your compiler places the variable fsp immediately  
> before
> buf on the stack, and strlen(buf) is 0. Yup, you get an infinite loop
> because p will always be assigned the address of fsp. I'm not quite  
> sure
> what happens when the stack variables are ordered in a different
> configuration, probably something bad?
>
> Here's a quick fix, but maybe someone would want to find a better one:
>
> $ svn diff src/main/util.c
> Index: src/main/util.c
> ==================================================================> ---
src/main/util.c     (revision 40876)
> +++ src/main/util.c     (working copy)
> @@ -694,7 +694,8 @@
>          R_fixslash(buf);
>   #endif
>          /* remove trailing file separator(s) */
> -       while ( *(p = buf + strlen(buf) - 1) == fsp ) *p = '\0';
> +       if(strlen(p))
> +           while ( *(p = buf + strlen(buf) - 1) == fsp ) *p =
'\0';
>          if ((p = Rf_strrchr(buf, fsp)))
>              p++;
>          else
>
> Best,
>
> Jeff
> -- 
> http://biostat.mc.vanderbilt.edu/JeffreyHorner
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>
>