Xapian devel - Oct 2006 - Re: [Xapian-commits] 7283: trunk/xapian-core/ trunk/xapian-core/net/

On Wed, Oct 04, 2006 at 01:27:20PM +0100, richard wrote:
> SVN root:       svn://svn.xapian.org/xapian
> Changes by:     richard
> Revision:       7283
> Date:           2006-10-04 13:27:17 +0100 (Wed, 04 Oct 2006)
> 
> Log message (2 lines):
> Use "unsigned char" instead of "char" when serialising
lengths, to avoid
> problems on platforms where char is signed (eg, windows).
Not just windows - char is also signed on x86 and x86_64 linux where I
wrote and tested this code...

I considered this issue while writing the code, and I don't think
there's actually a problem here.  What do you think will go wrong if
char is signed?

Cheers,
    Olly

On Wed, Oct 04, 2006 at 03:13:03PM +0100, Olly Betts
wrote:
> On Wed, Oct 04, 2006 at 01:27:20PM +0100, richard wrote:
> > Use "unsigned char" instead of "char" when
serialising lengths, to avoid
> > problems on platforms where char is signed (eg, windows).
> 
> Not just windows - char is also signed on x86 and x86_64 linux where I
> wrote and tested this code...
> 
> I considered this issue while writing the code, and I don't think
> there's actually a problem here.  What do you think will go wrong if
> char is signed?
Ah.

MSVC gives a warning: "serialise.cc(52) : warning C4309:
'static_cast' :
truncation of constant value".  I looked at the code and thought that this
meant that the 0x80 would be truncated to 0.  Of course, this is foolish,
and it actually gets converted to "-128".  I just wrote a quick test
case
to check that this is happening correctly on windows, despite the warning,
and it is, so perhaps I should revert the patch.

It would be nice to remove the warning on windows though.  Would
"static_cast<char>(-128)" be equivalent?  Or is the code after
my patch
still correct?

Sorry for the mess: I thought I'd ease into committing stuff to Xapian
again after a long break by doing some small uncontroversial fixes.  I
don't seem to be doing too well so far. :(

-- 
Richard