Re: C++ string::find functions and size_t

[Daniel J Sebald]

On 02/27/2015 06:30 PM, Rik wrote:
> On 02/27/2015 03:37 PM, Andreas Weber wrote:
> >  Am 27.02.2015 um 18:47 schrieb rik:
> > > 2/27/15
> > >
> > > Dan Sebald found a subtle bug in the use of the C++ string find functions
> > > that was leading to segfaults.
> > >
> > > -- Code --
> > > size_t pos = file.find_first_not_of ("|");
> > > if (pos > 0)
> > >   file = file.substr (pos);
> > > else
> > > -- End Code --
> >
> >  I think the idea here was to check if the first char in file is "|" and
> >  omit the check (because we are building a pipe) for an existent
> >  directory in the else path.
> >
> > > The issue is that the size_t is an unsigned quantity and the find
> functions
> > > do not return -1 on failure as do regular C functions.  Instead they
> return
> > > std::string::npos (a very large number) when they fail to find the
> search term.
> > >
> > > This was easily corrected to
> > >
> > > -- Code --
> > > size_t pos = file.find_first_not_of ("|");
> > > if (pos != std::string::npos)
> > >   file = file.substr (pos);
> > > else
> > > -- End Code --
> >
> >  Please correct me if I'm wrong but shouldn't this then be
> >
> >  -- Code --
> >  size_t pos = file.find_first_not_of ("|");
> >  if (pos != std::string::npos && pos > 0)
> >  -- End Code --
>
> I didn't check the purpose of the code, but if that is the case,
> shouldn't it be simpler to just write
>
> -- Code --
> if (! file.empty () && file[0] == '|')
>    file = file.substr (1);  // Strip leading pipe character
> else
> -- End Code --
>
> This avoids playing around with the position entirely.
>
> I made that change to both drawnow and in __osmesa_print__ and the
> latter now passes the built-in self tests.  See cset
> http://hg.savannah.gnu.org/hgweb/octave/rev/6b7aee95c54c

+          if (! file.empty () && file[0] == '|')
             {
               // create process and pipe gl2ps output to it
-              std::string cmd = file.substr (pos);
+              std::string cmd = file.substr (1);

These two cases will probably cause a problem here:

"|"
"|   "

In the first case there is only file.substr[0], no file.substr[1].  In 
the latter, well I guess it's just an empty command and not a seg fault 
and for the drawnow() command would end up being a 'broken pipe' because 
it's empty.  Again, only important if these are user-specified strings.

Dan