octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: warnings originating in gl2ps -- update


From: John W. Eaton
Subject: Re: warnings originating in gl2ps -- update
Date: Thu, 10 Feb 2011 12:36:18 -0500

On 10-Feb-2011, CdeMills wrote:

| I wrote a small test case reproducing the warning found in src/load-path.c,
| line 620. To be compiled as
| gcc -g -pedantic -Wsign-compare  -o test_sign test_sign.c
| 
| The good news: all comparisons gives the expected output, even in the case
| where the unsigned int has a value driving the signed int into overflow.
| To remove the warning, one option is to patch load-path.c like this:
| 
| diff -r 357d593d87c1 src/load-path.cc
| --- a/src/load-path.cc        Thu Feb 10 00:58:31 2011 -0500
| +++ b/src/load-path.cc        Thu Feb 10 16:11:40 2011 +0100
| @@ -617,7 +617,7 @@
|    while (k > 1 && file_ops::is_dir_sep (dir[k-1]))
|      k--;
|  
| -  if (k < dir.length ())
| +  if (((octave_idx_type)(k - dir.length ())) < 0)
|      dir.resize (k);
|  
|    return dir;
| 
| Should we apply this for each warning ?

I'd rather not.  First, the C-style cast you are using will result in
a different warning about that given our GCC warning options, and
second, I would still like to consider the possibility of having
different types for indices and dimensions (as does Matlab, with
mwIndex, mwSignedIndex, and mwSize), so some of the casts that
you would propose might become unnecessary.  And finally, I really
don't like to see casts, and especially not C-style casts, because
they will simply hide potential type mismatches that we should know
about.  So before you sprinkle the Octave sources with many more
casts, explain to me precisely what bad thing could happen in the
above code.  How large would k or dir.length () have to be before you
would see a problem with octave_idx_type either a 32- or 64-bit signed
integer?  Is that remotely likely to happen?  Is it even possible?

jwe


reply via email to

[Prev in Thread] Current Thread [Next in Thread]