stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: [PATCH] potentially bug in __catfind(), catalog.cpp
Date Sat, 15 Sep 2007 21:10:24 GMT
Farid Zaripov wrote:
>   I'm not sure to name this as bug. Let's it be a potentially bug.
> 
>   Below is the __catfind() function from catalog,cpp file:
> 
>   CatVector::size_type __catfind(nl_catd id)
>   {
>     CatVector::size_type i = 0;
>     while (i < __rw_catlist.size() && __rw_catlist[i] &&
> __rw_catlist[i]->id() != id)
>       i++;
>     if (!__rw_catlist[i])
>       return __rw_catlist.size();
>     return i;
>   }  
> 
>   If this function will be invoked with invalid id when __rw_catlist
> vector
> is filled up, then after while loop i == __rw_catlist.size() and we get
> undefined behavior in "if (!__rw_catlist[i])" line.

Sure looks that way. Good catch!

> 
>   I propose to rewrite this function this way:
> 
>   CatVector::size_type __catfind(nl_catd id)
>   {
>     for (CatVector::size_type i = 0; i < __rw_catlist.size() &&
> __rw_catlist[i]; ++i)
>       if (__rw_catlist[i]->id() == id)
>           return i;
>     
>     return __rw_catlist.size();
>   }  
> 
>   I can't write the regression test for this situation because the
> __catfind() is invoked
> in library always with valid id. And also I can't write the test which
> invokes __catfind()
> explicitly because of that function is inaccessible from user code.
> 
>   Here the patch:

Makes sense to me (remember the 78 character/line limit below).

Martin

> 
>   ChangeLog:
>   * catalog.cpp (__catfind): Fixed potentially undefined behavior when
>   __rw_catlist vector is full and id is not valid.
> 
> 
> Index: catalog.cpp
> ===================================================================
> --- catalog.cpp	(revision 575597)
> +++ catalog.cpp	(working copy)
> @@ -71,12 +71,11 @@
>  
>    CatVector::size_type __catfind(nl_catd id)
>    {
> -    CatVector::size_type i = 0;
> -    while (i < __rw_catlist.size() && __rw_catlist[i] &&
> __rw_catlist[i]->id() != id)
> -      i++;
> -    if (!__rw_catlist[i])
> -      return __rw_catlist.size();
> -    return i;
> +    for (CatVector::size_type i = 0; i < __rw_catlist.size() &&
> __rw_catlist[i]; ++i)
> +      if (__rw_catlist[i]->id() == id)
> +          return i;
> +    
> +    return __rw_catlist.size();
>    }  
>  
>  
> 
> Farid.
> 


Mime
View raw message