Hi all,

I've a problem with BeToN with smallints on a x86 system. When you use BeToN on smallint's with values like $8000 and $8200 it will return values like $FF80 and $FF82, instead or $0080 and $0082 respectively. The patch below fixes this, but I'm not sure if it's a correct fix, or that it's a bug in the compiler. (ie: is this behaviour of shr/shl normal on signed smallints?)

If this patch is correct, how about the other signed integer-types?

Patch:

--- rtl/inc/generic.inc (revision 10836)
+++ rtl/inc/generic.inc (working copy)
@@ -1707,7 +1707,7 @@
 {$ifndef FPC_SYSTEM_HAS_SWAPENDIAN}
 function SwapEndian(const AValue: SmallInt): SmallInt;
   begin
-    Result := (AValue shr 8) or (AValue shl 8);
+    Result := (word(AValue) shr 8) or (word(AValue) shl 8);
   end;

test-program:

program swap;
uses sysutils;
var si1,si2 : smallint;
begin
  si1 := $8000;
  si2 := $0080;
  si1 := beton(si1);
  if si1si2 then
    begin
    writeln('Not good:');
    writeln(IntToHex(si1,2),' should be ', IntToHex(si2,2));
    end;
end.

I don't know whether it's normal, but it's compatible with Delphi. I already fixed the same bug in r6752 (for swap() rather than for swapendian()).

I don't understand why we have both the swap() and swapendian() routines though, as this duplication (as demonstrated) can easily lead to unfixed bugs in one case or the other.

Jonas

I don't know whether it's normal, but it's compatible with Delphi. I already fixed the same bug in r6752 (for swap() rather than for swapendian()).

I think shr is supposed to work sign-extending with signed types. So it cant be used for endian swapping for signed types.

-Michael

The above statement may cause some confusion, so people who think that "shr" means "shift arithmetic right", please read the comment added to http://bugs.freepascal.org/view.php?id=10545

Jonas

OOPs !?!?!?!

They say: "SHR is /always/ unsigned in TP/FPC/Delphi."

This would mean that the patch the OP mentioned should not be necessary (but obviously it is and so there seems to be some bug somewhere).

The stuff about first converting and then shifting seems quite queer (unless the types left and right are different which imposes queer behavior in C, too).

-Michael

No, it doesn't. The reason is explained in the comment.

The stuff about first converting and then shifting seems quite queer (unless the types left and right are different which imposes queer behavior in C, too).

In Pascal, calculations are usually performed using the native integer type, regardless of the left and right types (there are some exceptions, but not many). That is not true for C.

Jonas

Sorry, didn't realize that when I wrote those. (maybe still some 16 bit artifact)
The reason that some of them are dups of swap is that imo the only usefull implementation of swap is the word version. This was valid enoug for me to implement all in a same way/name.

Marc

Op dinsdag 29-04-2008 om 15:53 uur [tijdzone +0200], schreef Jonas Maebe:

Thanks for fixing it. But coudn't you commit it 5 minutes earlier? Now it was too late for the postgres-testsuite run. Smallintfields still fails... ;)

Well, better luck tomorrow. ;)

Joost.